-
Notifications
You must be signed in to change notification settings - Fork 50.2k
Reconciler: Change commitUpdate signature to account for unused updatePayload parameter
#28909
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
Reconciler: Change commitUpdate signature to account for unused updatePayload parameter
#28909
Conversation
|
Comparing: cf5ab8b...34fd689 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
3db92a6 to
2989591
Compare
| // We may have an update on a Hoistable element | ||
| const updatePayload: null | UpdatePayload = | ||
| (finishedWork.updateQueue: any); | ||
| finishedWork.updateQueue = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed by logging in yarn test that finishedWork.updateQueu is always null here
| // TODO: Type the updateQueue to be specific to host components. | ||
| const updatePayload: null | UpdatePayload = | ||
| (finishedWork.updateQueue: any); | ||
| finishedWork.updateQueue = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed by logging in yarn test that finishedWork.updateQueu is always null here
jackpope
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One lint issue to fix
…datePayload` parameter
2989591 to
34fd689
Compare
diffInCommitPhaseis shipped (#27409) so we don't need to pass alongupdatePayloadwhich is alwaysnull.react-three-fiberwas bailing out if this payload wasnulland therefore never committed any update.By changing the signature the breakage becomes more obvious. The docs already document the proposed signature: https://github.com/facebook/react/blob/main/packages/react-reconciler/README.md#commitupdateinstance-type-prevprops-nextprops-internalhandle