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

RFC: Hot Module Replacement #257

Closed
JoviDeCroock opened this issue Dec 15, 2020 · 7 comments
Closed

RFC: Hot Module Replacement #257

JoviDeCroock opened this issue Dec 15, 2020 · 7 comments
Labels
architecture discussion Ongoing discussion / decision-making

Comments

@JoviDeCroock
Copy link
Member

JoviDeCroock commented Dec 15, 2020

State of the art

The state of the art with implementations like web-dev-server, snowpack and vite is following the esm-hmr spec.
This means that we have an hmr-client that lives on the client-side of the application, this hmr-client registers itself and modules to the server-side websocket.

When an update comes in the server will check whether or not the updated module is accepting updates, when it does it sends down the new copy with ?mtime=x to invalidate the cache and lets the accept function handle the rest.
When the module doesn't accept updates it'll bubble up to find the closest parent that does accept the update (think about boundaries).

The above is handled on the server so this means that it has the traversed chain in-memory, this means that if a non-updating child bubbles up to a grandparent or further the server can use its smarts and rewrite all the traversed imports with ?mtime=x so the accepting parent doesn't work with stale equivalents of our updated module.

Stale-child problem

With the above we've bumped into one problem, think about the following scenario, we have a file called parent.js and a child.js. The parent.js imports child.js and uses it as a util function.
We update child.js to return a value of 2 rather than 1, the module will be refetched, the cache will be busted due to ?mtime=x and we reexecute the function, the parent will get a value of 2 from this function now.

Now we update the parent.js file, this accepts the update, the module is fetched and the cache invalidated due to ?mtime=x. This will import from child.js which in our cache is still the old module which will return 1 rather than 2 this means that the wrong logic will be used with the updated parent.

Notice that with the above logic we always orphan the updated module.

Current wmr implementation

Currently in wmr we don't bubble for non-accepting children, we always expect a module to handle its own updates which can be hard for scenario's like Preact-hooks, hooks don't represent UI but the usage of said hook in a component does.

wmr currently handles everything on the client, which imposes several challenges, bubbling up is possible, we can visualize our module graph but updating children by appending ?mtime=x isn't possible due to us having no knowledge about the path we've traversed. We can't just invalidate all imports since this could get really costly.

We could try and check which module comes back and rewrite them on the client but in my opinion that isn't the cleanest way, I'm mainly thinking about whether or not we can get around the child-orphaning issue we're currently seeing in the esm-hmr implementation.

I've been trying some in-between solutions like sending along the chain of imports to wmr which could serve as a temporary fix.

Initial POC: #222

@43081j
Copy link

43081j commented Dec 16, 2020

When we tackled this in dev-server, from what I remember we added a server side transform which replaces imports with timestamped ones to bust the cache.

We also added the ability to specify that an update should continue bubbling even though you accepted it (to support some edge cases).

Would be interesting to see how you solve the cached import problem too for sure

@JoviDeCroock
Copy link
Member Author

JoviDeCroock commented Dec 16, 2020

@43081j yes exactly, which has proven to be a very good solution when we look at one update in isolation. When we go one level higher and look at this from multiple updates we risk stale modules due to the original code not containing these updated equivalents. (See: preactjs/prefresh#236)

We could implement some smarts that contains all updated modules but this could get quite expensive/chonky and seems to avoid the main problem. The server-side approach always orphans these newly updated modules and leaves the original cached for the original import-location, i.e. child.js will always refer to the original code in the byte-cache.

We could look at replacing ModuleRecords in the browser by fetching any module along the bubbled path and mutating the exports which in turn introduces a new issue now for non-exported/root modules since we wouldn't be able to detect these.

It's a pretty tough cookie and might refer to a missing primitive.

@43081j
Copy link

43081j commented Dec 17, 2020

if i've understood then, the problem is this:

- foo.js
--- bar.js

// update bar.js

- foo.js
--- bar.js?m=1234

// update foo.js

- foo.js?m=1234
--- bar.js // THIS is the old cached bar.js

and a map of what we've updated in the past could grow pretty quickly

@JoviDeCroock
Copy link
Member Author

JoviDeCroock commented Dec 17, 2020

Yes, that's the issue with the current approach. Prefresh and react-refresh currently handle this themselves, trying to see if we can fix this on a higher level.

Ref to how Prefresh takes care of this preactjs/prefresh#236

@43081j
Copy link

43081j commented Dec 22, 2020

I think one way or another you would need server state to know which modules have been updated in the past. The only way to avoid the browser's module cache is by changing URL. So even though a long lasting map in memory isn't great, I think most solutions will drill down to it ultimately.

An update could have happened any time earlier, not necessarily in the same update. So there has to be state like that

@JoviDeCroock
Copy link
Member Author

Yes, I guess our current approach is the best-possible for the time being until we have some sort of module-loader API.

@JoviDeCroock
Copy link
Member Author

Implemented in #262 the issue described in this RFC where a parent can get stale over time is fixed from within Prefresh/React-fast-refresh.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture discussion Ongoing discussion / decision-making
Projects
None yet
Development

No branches or pull requests

3 participants