-
-
Notifications
You must be signed in to change notification settings - Fork 320
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
"New props" arrow label is misleading #39
Comments
Agreed. "New props" is confusing and doesn't have any (correct) meaning. "Parent rerenders" give infomation about "why" and "when", just like the "setState()" and "forceUpdate()" labels do. |
Thanks for your input @csr632 ! In the meantime, I've had a colleague suggest "nextProps". Also, maybe "parent render()" would be preferable to "parent rerenders" to clarify the possible ambiguity between the React lifecycle render() method vs. the browser rendering. Not sure if Dan has any opinion about this... |
I feel that "parent render()" is perfect. When I give a course, I use this wording: |
Totally agreed! This arrow is very misleading, which confused me for quite a long time...for correctness, as long as parent component rerenders, child components should rerender as well (not if its shouldComponentUpdate returns false) |
Motivation
I realize that the phrase "New props" is in the original tweet of Dan Abramov, but "new props" is vague and misleading. Further, it tends to reinforce the misconception mentioned in Brian Vaughn's blog post that the "Updating" swimlane is triggered only when the props "change" or are "different" ( "new" props suggests "different" from "old" props). As the blog post says "these lifecycles are called any time a parent component rerenders, regardless of whether the props are 'different' from before".
Proposal
Change the wording to say "parent rerenders" or something similar.
Aside
I'm not trying to be picky. Dan's diagram and your project have been infinitely helpful in aiding my understanding, yet I've continuously seen beginner, intermediate, and even some advanced React developers tripped up by this confusion. It's a small change, but if you agree, I can help out and do the PR as well.
The text was updated successfully, but these errors were encountered: