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

React.createContext singleton enforcement? #13346

Closed
rtsao opened this issue Aug 8, 2018 · 3 comments
Closed

React.createContext singleton enforcement? #13346

rtsao opened this issue Aug 8, 2018 · 3 comments

Comments

@rtsao
Copy link

rtsao commented Aug 8, 2018

Do you want to request a feature or report a bug?

A bit of both?

What is the current behavior?

The old context API involved components that provided and consumed context using a shared string key. In other words, the only requirement for communication between components providing or consuming context was that the string key used was the same. This had the side effect of allowing easy communication via React context across packages (i.e. separate node_modules using the same shared context). This might be one of two forms:

  1. Entirely different components (using the same context key), perhaps spread around the source code or even in node_modules
  2. One or more packages importing the context providing/consuming components of another (a usage pattern more in line with the React.createContext abstraction).

The new React.createContext API works much differently. Instead, for communication to work correctly, the consumer and provider must always come from the exact same function call. Unlike the old API, this means if the provider or consumer is imported from another package, there is now an implicit singleton, otherwise there will be separate React.createContext instances.

I think it is intuitive and reasonable that "form 1" does not work with the new API, but less so with "form 2".

What is the best way of handling this? So far, the best approach I've thought of is using peer dependencies to enforce the package calling React.createContext is never duplicated in node_modules.

This potential drawback was not brought up in the original React.createContext RFC and I can't seem to find any reference to this limitation in the docs anywhere.

Is there a recommended way of handling this? Should this be listed in the documentation as a caveat when using React.createContext? Is there a way to make this less error-prone? There didn't seem to be an issue regarding this already so I figured I'd create one and kick off a discussion.

What is the expected behavior?

Context communication should work across package boundaries (see above note on old context API)

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

This concerns versions of React with the React.createContext API

@gaearon
Copy link
Collaborator

gaearon commented Aug 8, 2018

In other words, the only requirement for communication between components providing or consuming context was that the string key used was the same.

Yeah. The upside of this is it's easier to share context intentionally but the big downside is that it's easy to have context clashes from components that don't actually want the same context.

What is the best way of handling this? So far, the best approach I've thought of is using peer dependencies to enforce the package calling React.createContext is never duplicated in node_modules.

That sounds reasonable to me, just like these libraries already have a peer dependency on React itself.

@fzaninotto
Copy link

fzaninotto commented Jul 25, 2019

I fell on this one by chance, and I'm surprised the gotcha isn't advertised more prominently.

It seems to me that React contexts are ubiquituous: react-redux, react-router, material-ui, react-final-form all use the new Context API. It means they should never be added as dependencies, but only as peerDependencies, yet none of these libraries explain that. I don't think it's the fault of the maintainers of these packages - I believe they're just ignorant of the problem.

My opinion is that it's React's responsibility to warn against duplicate contexts due to duplicate dependencies, or to make new contexts singletons (as the old contexts). Just like the commenter on reactjs/react.dev#1112 (comment), I think the latter is a better solution because that's what developers assume.

@eps1lon
Copy link
Collaborator

eps1lon commented Jul 25, 2019

I believe they're just ignorant of the problem

Well aware of this. The upside is that you get an additional warning for module duplication. For versions with the same major version you want a single version most of the time anyway. Across major versions you specifically don't want to share context because they might be incompatible.

It means they should never be added as dependencies, but only as peerDependencies, yet none of these libraries explain that.

We usually ask other packages depending on @material-ui to list us as peer dependencies (e.g. mbrn/material-table#472). We even document how rogue dependencies might cause harm to your bundle. If they don't and we still list them in our docs feel free to open an issue on our repository.

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

4 participants