Context providers and consumers should bailout on already finished work#12254
Conversation
Fixes bug where a consumer would re-render even if its props and context had not changed.
|
@gaearon Ready for review |
gaearon
left a comment
There was a problem hiding this comment.
Verified it fixes my issue. Code seems to make sense.
| } else if (oldProps === newProps) { | ||
| return bailoutOnAlreadyFinishedWork(current, workInProgress); | ||
| } | ||
| } else if (changedBits !== 0) { |
|
This change seems to make sense to me. 👍 |
|
@sebmarkbage I'm gonna merge this as-is so I can start the sync. If you have additional feedback, I'll fix in a follow up. |
| 'A context consumer expects a single child that is a function. ' + | ||
| 'If you did pass a function, make sure there is no trailing or leading whitespace around it.', | ||
| if (__DEV__) { | ||
| warning( |
There was a problem hiding this comment.
Should we move this to context creation then? Or do we already warn there?
There was a problem hiding this comment.
Context creation is too early. Needs to check on every render. Like a prop type.
There was a problem hiding this comment.
I mean createElement when we know type is a context consumer.
There was a problem hiding this comment.
Ah I see. Idk, not sure what the advantages are. I thought about using propTypes but if the plan is to remove those eventually? Although maybe this is an indication that would shouldn't...
There was a problem hiding this comment.
Idk, not sure what the advantages are.
You get a JS stack trace pointing where it happened.
…rk (react#12254) * Context providers and consumers should bail-out on already finished work Fixes bug where a consumer would re-render even if its props and context had not changed. * Encode output as JSON string * Add weights to random action generator * Add context to triangle fuzz tester * Move bailouts to as early as possible * Bailout if neither context value nor children haven't changed (sCU) * Change prop type invariant to a DEV-only warning
…rk (react#12254) * Context providers and consumers should bail-out on already finished work Fixes bug where a consumer would re-render even if its props and context had not changed. * Encode output as JSON string * Add weights to random action generator * Add context to triangle fuzz tester * Move bailouts to as early as possible * Bailout if neither context value nor children haven't changed (sCU) * Change prop type invariant to a DEV-only warning
Fixes bug where a consumer would re-render even if its props and context had not changed.