-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Add failing test for #17213 #17336
Add failing test for #17213 #17336
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 5fb7b0c:
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you! |
Thank you for the test! Took us a while to get to this. Sorry this got autoclosed. |
Thanks @gaearon ! Andrew told me as well: #17314 (comment) :D |
I've tried to add a failing test for the issue described here.
#17314
If you run it locally, and change
isMemo
to be false, the test will pass. The test will also pass if you change the line toconst MemodSubbed = isMemo ? React.memo(Subbed, () => true) : Subbed;
Below are my half-hearted attempts to look into this bug.
It looks like when the
compare
function is defined it makes React callcreateFiberFromTypeAndProps
. However, if thecompare
function is not defined, it means React goes down the codepath below;https://github.com/facebook/react/blob/master/packages/react-reconciler/src/ReactFiberBeginWork.js#L380-L403
In other words, the bug is likely caused somewhere in
updateSimpleMemoComponent