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

Clarify that createContext calls should be in separate files, and not in same file as Provider/parent #1481

Closed
JReinhold opened this issue Dec 11, 2018 · 1 comment

Comments

@JReinhold
Copy link

This issue was created on recommendation from @gaearon

The problem

This issue originates from facebook/react#13969, so to get the full story, you should read that issue.

The short version is that there is a great chance of getting into a circular dependency problem, if the developer defines the context in the same file as the Provider is added, instead of doing it in a separate file. If this happens, the parent with the context will import some child that imports the context to create the Consumer, and this is a circular dependency: parent-with-context -> ... -> descendant-that-needs-context -> parent-with-context.

What happens in practice, is that the context becomes an empty object, which is really confusing if you don't know the internals of dependencies, imports, etc.

This is not really explained well in the docs. in the Context - Examples section, the context is correctly created in a separate theme-context.js file, but it is not specified that it has to be done that way.

Proposed solutions

  1. Add a warning box to the Context - React.createContext section, stating that it has to be done in a separate file, and why
  2. Divide the Context - Caveats section into subsections, and make a subsection clarifying the problem and solution.

I prefer the second solution, especially because:

IMO we should warn if it looks like a circular dependency (empty object or undefined). - @gaearon - facebook/react#13969 (comment)

If we warn the developer, I think the best way to do it would be to add a link to a specific subsection in the docs, rather than a warning box.

I could take a stab at a PR for a change to the docs, if the React team agrees with this issue.

@JReinhold
Copy link
Author

Closing in favor of facebook/react#15142 and #1842

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

No branches or pull requests

1 participant