-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[docs][table] Stabilize random series in virtualized table demo #43744
Conversation
Netlify deploy previewhttps://deploy-preview-43744--material-ui.netlify.app/ Bundle size report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In MUI X, we use two different solutions for this problem:
- Data Grid: https://github.com/mui/mui-x/blob/d64e37732f1384e54727a4f013a66bfdacfa36b7/packages/x-data-grid-generator/src/services/random-generator.ts#L20 used with https://github.com/mui/mui-x/blob/d64e37732f1384e54727a4f013a66bfdacfa36b7/test/regressions/webpack.config.js#L25
- Charts: https://github.com/mui/mui-x/blob/d64e37732f1384e54727a4f013a66bfdacfa36b7/docs/data/charts/axis/MinMaxExample.js#L7
Maybe we could do the same here, use Chance(42)
straight in the demo?
would be nice to have this abstracted, but compared to what I wrote here, it feels a bit excessive to pull in as a dependency in the docs 🤔 Edit: There's an open issue for quite some time 🙂 I'll update the test, not going to do |
True, chance.js a big dependency but look at that https://bundlephobia.com/package/@faker-js/faker@9.0.1. God 3MB https://esm.sh/v135/@faker-js/faker@9.0.1/es2022/faker.mjs at that point it's a joke 😂. Looks good |
Make sure we always generate the same list so that it stays stable during screenshot testing. Changes the demo to something that's more suitable to generate with
chance
and usesnew Chance(42)
to seed its random number generator to make it produce a stable sequence.In X this is seeded it with
() => 0.5
and it has aDISABLE_CHANCE_RANDOM
, but this feels highly unnecessary, I've left it out for the core implementation.Before: https://deploy-preview-43740--material-ui.netlify.app/material-ui/react-table/#virtualized-table
After: https://deploy-preview-43744--material-ui.netlify.app/material-ui/react-table/#virtualized-table