-
-
Notifications
You must be signed in to change notification settings - Fork 32.1k
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
[LeftNav] move unnecessary state to instance properties. #1184
Conversation
@pomerantsev What do you think about this change? |
Actually, I was just used to storing any local state properties on the |
@pomerantsev use React.tools.Perfs to measure performance. From what I understand about reading the code, the changeset make a lot of sense. State should only include props that trigger a rerender or a lot of rerender are done for nothing. (and there are many many useless rerender in mui at the moment, it would be nice to lower them) Optimally when the render method is called it must produced a different output than the previous call. |
to say it in the reverse way, state should only includes properties used in the render function. |
@cgestes: cool, I'll take a look at the Perf stuff. I totally understand what you're saying about the |
I agree with @cgestes the state should only contains properties used in the render method. |
Agree with @cgestes. |
Great. @maoziliang, thanks for the optimization! |
Thanks @maoziliang @cgestes @pomerantsev @oliviertassinari - you guys are awesome! 👍 |
[LeftNav] move unnecessary state to instance properties.
* Correct position of inline wrapper popover * Adjust variant prop description * Adjust props description * Uncomment example * Add prop-types.json to prettier-ignore * Properly freeze clock for percy
For performance.
I think the properties that not trigger component's render should not belong to component's
state
.This can reduce many render cycles.