[docs] Memoize array sorting

[iammminzzy]

• I have followed (at least) the PR section of the contributing guide.

Address #41709 (review)

[mui-bot]

Netlify deploy preview

https://deploy-preview-42010--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 4f73f1a

[ZeeshanTamboli]

Do we really need to optimize performance with memoization here? The tables have only 15-20 rows, except for the MaterialShowcase demo which has around 45 rows. Most of these useMemo have dependencies that frequently change. Except for when a row is selected or all rows are selected (updating the selected state) in which case, the component would re-render, allowing for reusing the cached rows value. What are your thoughts? @Janpot

[NMinhNguyen]

Cc @michaldudak (#41709 (comment))

For what it’s worth, there’s already a precedent:

material-ui/docs/data/material/components/table/EnhancedTable.tsx

Lines 305 to 311 in ca6b5b4

	const visibleRows = React.useMemo(
	() =>
	[...rows]
	.sort(getComparator(order, orderBy))
	.slice(page * rowsPerPage, page * rowsPerPage + rowsPerPage),
	[order, orderBy, page, rowsPerPage],
	);

so whatever direction this PR goes in should probably be consistent overall.

[Janpot]

Do we really need to optimize performance with memoization here? The tables have only 15-20 rows, except for the MaterialShowcase demo which has around 45 rows.

This is mostly a philosophical argument, but I think in our examples we have the duty to write idiomatic React code. Our community copies and pastes from them and takes inspiration. It is technically true that in the minimal example, memoizing won't move the needle significantly in terms of performance, if it moves at all. But most derivatives of this example will turn out to be vastly more complex. While I don't want to open up the debate between "memoize all" vs. "measure before memoize", I feel like there would be a relatively broad consensus around always memoizing if the operation is >=O(n), especially if the result is not referentially stable. It's a biased viewpoint though, I don't have the bandwidth to engage in a follow-up discussion 🙂.

[ZeeshanTamboli]

Okay, in that case, I'll leave the final decision to official team members like @DiegoAndai or others.

[DiegoAndai]

I usually agree that demos should include only the strictly necessary. @Janpot makes a good point that the useMemo feature will benefit the majority of users' use cases, although this can also be "covered" by adding a callout in the docs about memoization under the demos that might benefit, like these. This would be more educational and better guidance for users than blindly copying useMemo without understanding why, but people might not read it.

I'm not sure what's the better option 😅, but I agree it should be consistent. I would like to know @samuelsycamore's take on this.