Skip to content
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

[Table] Use stable context API #13529

Merged
merged 4 commits into from Nov 8, 2018

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 6, 2018

Part of #13394.

Only form and theme context left. Theme will be addressed by #13503. I'm working on the form.

@eps1lon eps1lon added the component: table This is the name of the generic UI component, not the React module! label Nov 6, 2018
@eps1lon eps1lon added this to the v3.5.0 milestone Nov 6, 2018
}
function TableBody(props) {
const { classes, className, component: Component, ...other } = props;
const childContext = { variant: 'body' };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an easy perf optimization I would suggest moving this to a string only. Otherwise a rerender of TableBody will trigger a rerender of every context consumer since strict equality is used for rerender check.

Already included the perf optimization
@oliviertassinari oliviertassinari force-pushed the fix/core/Table/stable-context branch 2 times, most recently from c2aa160 to a964103 Compare November 7, 2018 22:33
@oliviertassinari
Copy link
Member

@eps1lon A quick benchmark on server side. I think that we are good to go.

  • master x 151,736 ops/sec ±6.79% (165 runs sampled)
  • two context x 135,449 ops/sec ±5.17% (184 runs sampled)
  • one context x 134,876 ops/sec ±6.68% (181 runs sampled)

@oliviertassinari oliviertassinari merged commit be89b0a into mui:master Nov 8, 2018
@oliviertassinari
Copy link
Member

Good job here!

@eps1lon eps1lon deleted the fix/core/Table/stable-context branch December 5, 2018 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: table This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants