Open elliottkember opened 4 years ago
Did you provide your app with a Provider? Sorry for the gioco di parole lol. Anyway, could you show us some code or even reproduce it to a codesandbox?
e.g.:
function App() {
return (
<Counter.Provider>
<CounterDisplay />
<Counter.Provider initialState={2}>
<div>
<div>
<CounterDisplay />
</div>
</div>
</Counter.Provider>
</Counter.Provider>
)
Hi! Great wordplay and I learned a new expression!
Yes, the app has a provider. To clarify, the container works great with hot loading until I change the code in the container. At that point i suspect it unmounts and remounts and loses context. I’m nesting 3 containers, it doesn’t happen for all of them, but it’s a container that I’m using in multiple nested components that’s causing problems.
I’m away from my computer for the next few days, then I will try and make a code sandbox that demonstrates this - but I don’t think the sandbox will hot-load components with react-hot-loader!
If it does not work, try with hook composition https://codesandbox.io/s/unstated-store-provider-k4h77?file=/src/store/index.js
Oh, that looks interesting - I’ll try it and let you know. Thanks!
I'm seeing this same issue with webpack hot module replacement (using create-react-app
's builtin hot reloading start
command). Not surprisingly, it appears to be an issue with React.useContext
itself, rather than strictly an issue of unstated-next
, though it does significantly impede usability of this library (and others that use React contexts).
Specifically, from my observations, it seems to happen when the state container definition is in the same module/file as the visual components that use that state.
So with that theory, an obvious workaround is to make sure that my state container is defined in a file separate from everything else with minimal import
dependencies. I was previously using defining small state containers alongside the visual components that used them, since I found it convenient to have those typescript types defined in the same file. However, since moving them to separate files I am no longer seeing this bug.
It may be possible for someone to come up with a fancier workaround using something like module.hot.accept(...)
to define some custom behavior that happens on such a reload, to preserve the React context. At the moment I'm not planning to go down that rabbit hole, but if someone else has success with that, let me know.
I came up with a minimal test case that shows this problem: https://github.com/elliottkember/unstated-hot-reload-test
If you update the container file, you'll find that it crashes this way.
To replicate this, clone that repository,
yarn install
npx parcel index.html
container.js
You should see that error message. It seems as though the container is being hot-reloaded but not all components that use it get the new component so the containers don't match. This is consistent for me under Parcel 1 and 2.
I don't seem to have the same issue when I migrate that test bed repo to Webpack. The root cause may be something that Parcel is doing with hot module reloading.
The one thing I've found that sort of alleviates this issue is this:
const container = useContainer(require("./container").default);
If I use a dynamic require to load my container in the component, when hot-reloading the container, the Provider and Component both get updated at the same time and will refer to the same Context object. I don't think this is intended.
It seems as though React is replacing the component that requires this container, at the same time as it replaces the component that uses the Provider. This means that the component gets a null
context when it's mounted, as the provider isn't yet available through the context API.
I've tracked this all the way through my application, and if I import
the container from two different components, it triggers this problem.
I don't really know how to fix this, and this dynamic require()
doesn't feel like the right solution. I'm hoping there's something we can do about this other than just reloading the whole page whenever this type of error is detected.
I've come up with a kooky little hack that works in some situations. I think it's related to context trees and loading order with hot-module-reloading. I've patched my own version of unstated, and I'm doing this (isDev is process.NODE_ENV === 'development'
)
After the value is called from useHook
, I set a cache key against an object I patch onto React:
function Provider(props) {
var value = useHook(props.initialState);
if (isDev) {
if (!React.__containerCache) React.__containerCache = {};
React.__containerCache[useHook.name] = value;
}
return React.createElement(
Context.Provider,
{ value: value },
props.children
);
}
Then I use that in useContainer
instead of just going straight to throwing the error.
function useContainer() {
var value = React.useContext(Context);
if (value === null && isDev) {
value = React.__containerCache[useHook.name];
}
if (value === null) {
throw new Error("Component must be wrapped with <Container.Provider>");
}
return value;
}
This has allowed me to hot-reload some of my containers. I will update this thread if this turns out to break everything, but it shouldn't affect anything in production, and has helped my workflow!
I am also experiencing similar issues. : (
I'm using
react-hot-loader
, and whenever I update a container, I get the above error. It seems to happen when a sub-component uses the container that's provided by a parent component. I can't figure out how to fix this.It doesn't seem to matter whether I use MyContainer.useContainer() or useContainer(MyContainer) - still the same result.