Skip to content

Fix incorrect legacy context for factory components#13441

Merged
gaearon merged 4 commits into
react:masterfrom
gaearon:factories-are-the-worst
Aug 20, 2018
Merged

Fix incorrect legacy context for factory components#13441
gaearon merged 4 commits into
react:masterfrom
gaearon:factories-are-the-worst

Conversation

@gaearon

@gaearon gaearon commented Aug 20, 2018

Copy link
Copy Markdown
Collaborator

@bvaughn bvaughn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Factory components are hard for me to think about for some reason. I think this fix looks okay though! Left a tiny nit about naming, but it's up to you whether you respond to it.

function getUnmaskedContext(
workInProgress: Fiber,
Component: Function,
didPushOwnContext: boolean,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: The naming of this implies that the Fiber is a context provider when it's actually meant more to reflect which phase the component is in. Maybe we could call it something more meaningful? Even didPushOwnContextIfProvider?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good. I couldn't find a name I'd be happy with. Which reflects how messed up this whole thing is.

@gaearon
gaearon merged commit 0beb2ee into react:master Aug 20, 2018
mrizwanashiq pushed a commit to mrizwanashiq/react that referenced this pull request Jun 25, 2026
* Add a repro case for context bug

* Be explicit about whether context has been pushed

* Put cheaper check first

* Naming
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants