-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Remove lifecycles from core #4768
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
Conversation
📊 Tachometer Benchmark ResultsSummaryA summary of the benchmark results will show here once they finish. ResultsThe full results of your benchmarks will show here once they finish. |
4cbe3fa to
259dfe0
Compare
|
Size Change: -120 B (-0.26%) Total Size: 46.4 kB
ℹ️ View Unchanged
|
ce917e1 to
230e39a
Compare
|
Going to close this for the time being as the win is small but we might want to revisit this |
|
Super late here, but I think we should go for it or revert dropping support for the React 16.3 allowed & encouraged users to start using these prefixes and React 17.0 required used of them; as such, now that we've dropped support for the prefix in #4656, we only support 16.x components (and older) that never underwent the optional migration. This feels like a somewhat awkward middle ground for support and might be surprising to some users, if any are still using these methods, that is. If they aren't, well, |
|
@rschristian I support that, let's do it when we decide whether we revert deferred state settling though as the previous iteration does use componentWillUpdate 😅 |
|
Ah, that's why there's that revert commit, forgot we used to use that! |
Checking out the byte size impact of removing these lifecycles, we can remove
componentWillUpdateas well due to #4760. 53b is a lower impact than I had expected, what do we think about this one?