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

Fix cDU to correctly make async request #745

Merged
merged 1 commit into from
Mar 30, 2018
Merged

Conversation

nateq314
Copy link
Contributor

@nateq314 nateq314 commented Mar 30, 2018

When a new id prop is passed in, getDerivedStateFromProps sets this.state.externalData to null. If I understand the new API correctly the return value of gDSFP() either becomes the new state or is merged into the prev state? If so, then componentDidUpdate should examine the new state (this.state.externalData), not prevState.externalData. Otherwise the async request will never happen.

When a new `id` prop is passed in, `getDerivedStateFromProps` sets `this.state.externalData` to `null`. If I understand the new API correctly the return value of gDSFP() either becomes the new state or is merged into the new state? If so, then `componentDidUpdate` should examine the new state (`this.state.externalData`), not `prevState.externalData`.
@facebook-github-bot
Copy link
Collaborator

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@reactjs-bot
Copy link

Deploy preview for reactjs ready!

Built with commit 51a864d

https://deploy-preview-745--reactjs.netlify.com

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

Whoops! 😅 Nice catch. Thanks!

@bvaughn bvaughn merged commit 85f3666 into reactjs:master Mar 30, 2018
@nateq314
Copy link
Contributor Author

Glad to help! Unrelated question: Any suggestions for familiarizing oneself with or attempting to wrap ones head around the React codebase? I'm really interested in understanding the internals and contributing somehow in the future but it's so intimidating in terms of sheer lines of code. Not even sure where to start. Other than taking a core package like react-reconciler and proceeding directly to a line by line deep dive, is there some other way to get a high-level understanding of the major components/structures and their relationships? For example stuff like, what exactly is a fiber, what is their structure and behavior and how do they interact. How does the upcoming async-mode work in terms of prioritizing/scheduling updates from a high level whiteboard point of view. How does reconciliation work. Etc etc. But with a focus on code and internals, not the API doc stuff on reactjs.org. Any tips you might have? Just asking. No offense if you don't have time to answer. Know you guys are all super busy between work and family.

@gaearon
Copy link
Member

gaearon commented Mar 31, 2018

There’s an outdated document here but it’s pretty old and some things changed. https://github.com/acdlite/react-fiber-architecture

There’s some background that’s still relevant here. facebook/react#7942

This might be helpful. https://engineering.hexacta.com/didact-fiber-incremental-reconciliation-b2fe028dcaec

Other than that we don’t currently have a lot of info but I’d encourage you to look at five files:

  • ReactFiberBeginWork (“entering” a component)
  • ReactFiberCompleteWork (“leaving” a component)
  • ReactFiberCommitWork (flushing changes to DOM)
  • ReactFiberScheduler (choosing next component to work on)
  • ReactChildFiber (diffing insertions/deletions of children)

I’m happy to answer questions (perhaps in a separate issue) and it would be great if you could write a doc with your learnings 🙂

@nateq314
Copy link
Contributor Author

nateq314 commented Apr 4, 2018

Thank you! Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants