Improve DataTable re-rendering performance

alexzherdev commented 2 years ago

Additional description

This improves re-rendering performance of the DataTable in scenarios involving many rows (in the hundreds/thousands). Main optimizations introduced:

<DataTable>: memoized TableContext value so that if table gets re-rendered with no real changes (e.g. when its parent re-rendered and didn't pass any new props), it doesn't trigger unnecessary re-renders in the context consumers
<DataTable>: memoized assistive text and columns passed down to rows to reduce their re-renders
<DataTableRow>: any change in TableContext would mark all rows for re-rendering due to useContext in the row. Implemented option 3 here to stop unnecessary re-rendering below the row (with the exception of relevant nested context consumers)
context-helper: turned it into a hook to enable useCallback. This was necessary so that the functions returned from the helper could be properly used as useMemo dependencies elsewhere.

Note that to take advantage of these improvements, the consumer application needs to memoize the columns passed into the table:

const columns = useMemo(() => [
  <DataTableColumn key="0" />,
  <DataTableColumn key="1" />
], []);

<DataTable>{columns}</DataTable>

Traces below are from a consumer application showing reduced re-rendering when clicking a row (these are taken from the re-render caused by the focus change in the table). Overall rendering time (measured via Chrome's profiler) decreased by 40-50%.

Before

After

CONTRIBUTOR checklist (do not remove)

Please complete for every pull request

[ ] First-time contributors should sign the Contributor License Agreement. It's a fancy way of saying that you are giving away your contribution to this project. If you haven't before, wait a few minutes and a bot will comment on this pull request with instructions.
[x] npm run lint:fix has been run and linting passes.
[x] Mocha, Jest (Storyshots), and components/component-docs.json CI tests pass (npm test).
[ ] Tests have been added for new props to prevent regressions in the future. See readme.
[x] Review the appropriate Storybook stories. Open http://localhost:9001/.
[x] Review tests are passing in the browser. Open http://localhost:8001/.
[x] Review markup conforms to SLDS by looking at DOM snapshot strings.

REVIEWER checklist (do not remove)

[ ] CircleCI tests pass. This includes linting, Mocha, Jest, Storyshots, and components/component-docs.json tests.
[ ] Tests have been added for new props to prevent regressions in the future. See readme.
[ ] Review the appropriate Storybook stories. Open http://localhost:9001/.
[ ] The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
[ ] Review tests are passing in the browser. Open http://localhost:8001/.
[ ] Review markup conforms to SLDS by looking at DOM snapshot strings.
Required only if there are markup / UX changes
[ ] Add year-first date and commit SHA to last-slds-markup-review in package.json and push.
[ ] Request a review of the deployed Heroku app by the Salesforce UX Accessibility Team.
[ ] Add year-first review date, and commit SHA, last-accessibility-review, to package.json and push.
[ ] While the contributor's branch is checked out, run npm run local-update within locally cloned site repo to confirm the site will function correctly at the next release.

interactivellama commented 2 years ago

I can see a noticeable difference on a table of 250 rows (7 columns) when selecting a row! 🎉

interactivellama commented 2 years ago

@alexzherdev Please merge when ready.

salesforce / design-system-react