-
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
Limitations of React.createContext #13773
Comments
Sorry, I might be missing something but I've read the question you linked to and I still don't understand what's being asked there. Can you provide a small example showing what you're trying to do? |
To be super clear, "default value" is what you get when you don't have a Provider at the top. If you want to override a value that children receive, you don't need to "create context inside" — you need to render a Provider with the value driven by your state. |
I replied to that question. Hope it helps. https://stackoverflow.com/a/52648865/458193 But in general, this seems unnecessary because usually you don't want to create new contexts per component. Instead, you want to use the Provider. |
@gaearon I imagine coupling the Context initialization to a single Provider will be the pathway that 99% of end users take. If that is how Redux is integrating, then I imagine most will follow. It doesn't feel correct that the right way to do this is to wrap it in a function or initialize the Context with null. The happy path doesn't feel that happy here. As always, thank you for the input. |
Sorry, I don't understand what we're talking about. I think we're probably talking about different things. Again, if you provide a specific example, I can look. |
@gaearon Sure thing. Here's a sample of the Provider from the React Redux PR: class Provider extends Component {
constructor(props) {
super(props)
const {store} = props
this.state = {
storeState : store.getState(),
store,
}
}
...
render() {
const ContextProvider = this.props.contextProvider || ReactReduxContext.Provider
return (
<ContextProvider value={this.state}>
{this.props.children}
</ContextProvider>
)
}
} and the context: export const ReactReduxContext = React.createContext(null); Flow doesn't like this, so we duplicated the Provider initial state. That feels odd to me. |
why would you pass in a provider via props? If redux does go with this, i don't think it will the 99% or even the common case for context using APIs. The large majority of them just expose the provider and have you render it at a top level, I can't really think of why you'd want to do the above...or for that matter why it isn't flow compatible. |
@jfo84 I didn't mean React Redux use case (honestly I haven't looked at why they're doing it), but I meant what's your specific use case? |
I completely agree with passing the provider as props. I think it's a bit strange. However, we do something similar to everything else you see above. For Flow, we're using a typed JS Object so it throws on accessing the keys. For a quick fix, we duplicated the Provider's state for the default value, passing in noop functions in place of functions defined as class properties on the Provider. We're removing some usages of Redux with small footprints and moving them over to Context, attempting to duplicate the API in the process. We went with what felt similar enough to Redux: a single Provider with state that mirrors the context and functions defined as class properties on the Provider to emulate dispatched actions. |
Sorry, I mean that I still don't understand why just defining context in top scope is not sufficient for you. A small example would really help. |
Okay @gaearon here's come code export type AnalyticsContextT = {|
startDate: Moment,
endDate: Moment,
blueprintId: string,
selectBlueprintId: Function,
changeDate: Function,
|};
const AnalyticsContext = React.createContext<AnalyticsContextT>({
startDate: moment().subtract(1, 'months'),
endDate: moment(),
blueprintId: '',
selectBlueprintId: noop,
changeDate: noop,
});
class AnalyticsProvider extends React.Component<Props, AnalyticsContextT> {
state = {
startDate: moment().subtract(1, 'months'),
endDate: moment(),
blueprintId: '',
selectBlueprintId: (id: string) => this.setState({ blueprintId: id }),
changeDate: (dates: { startDate: Moment, endDate: Moment }) => this.onChangeDate(dates),
};
render() {
const { children } = this.props;
return (
<AnalyticsContext.Provider value={{ ...this.state }}>
{React.Children.only(children)}
</AnalyticsContext.Provider>
);
}
} |
@jfo84 : I wrote that PR, so let me add some, uh, "context" to the discussion :) The default use case for React-Redux is a single store, and a However, all of the versions so far have also supported passing a Redux store instance directly as a prop to a connected component, like: const store1 = createStore();
const store2 = createStore();
ReactDOM.render(
<Provider store={store1}>
<App>
<ConnectedComponent1 />
<ConnectedComponent2 store={store2} />
</App>
</Provider>,
document.getElementById("root")
) This works so far, because each individual connected component is a separate subscriber to a Redux store. If you pass a store in as a prop, that component subscribes directly to the prop store instead of the store passed down through legacy context. In v6, we're switching from individual per-component subscriptions to a single store subscription, all the way up in the React-Redux As a potential alternative, we're trying out the idea of accepting a separate provider/consumer pair if you'd like to customize the behavior (like having a "sub-app" inside the main application). Let me know if you've got any further questions related to this. |
I'll also note that I've at least considered the idea of using two provider/consumer pairs. The first would be a singleton, and we'd create the second when |
Thanks for the input @markerikson. I find your second comment fascinating. I hope @gaearon Purely from a userspace perspective, could the |
@jfo84 : I'm with Dan - I still don't understand the actual concern you're trying to raise here. The default value for a context instance only gets used if you render a consumer with no provider above it. Since you'd normally be actually rendering a provider and giving it a value, the default value should be irrelevant 99.9% of the time. I don't have this in my 995 branch atm, but it would be reasonable for us to check if the value is null and throw an error saying "you've probably forgotten to put a React-Redux |
I understand the default value is unimportant 99.9% of the time, which is why I want it to be optional. Overriding |
It is optional? |
@jquense Flow fails when you consume the context AFAIK |
That's why |
We've just published React-Redux 6.0 beta 1, which uses |
Do you want to request a feature or report a bug?
feature
What is the current behavior?
The current behavior requires end users to use
createContext
in the module scope. To my understanding, it's not currently possible to use a default value derived from the state of a component (a stateful Provider in my case).This StackOverflow post hits the issue right on IMO.
I feel like this is the classic use case for replacing Redux, and it doesn't work out of the box with static types.
I think it's quite telling that
react-redux
is doing something similar here in their PR to move to React 16 context. I would expect the default value to bethis.state
of the Provider component instead ofnull
.My knowledge of React internals is naive, but I didn't see anyone else bringing up this issue.
If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:
What is the desired behavior?
Maybe a JSX API for context creation? I imagine it's not quite that simple.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
16.3+
The text was updated successfully, but these errors were encountered: