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

Implement full HMR in snowpack build --watch mode #1935

Open
FredKSchott opened this issue Dec 11, 2020 · 12 comments
Open

Implement full HMR in snowpack build --watch mode #1935

FredKSchott opened this issue Dec 11, 2020 · 12 comments
Labels
3.1 contributors welcome! contributors welcome!

Comments

@FredKSchott
Copy link
Owner

Original Discussion: #1459 #1008
/cc @joshnuss @francislavoie

snowpack build --watch. --hmr is working BUT it looks like it only supports basic browser-sync functionality. On change, the browser is automatically refreshed but import.meta.accept() is never called and full HMR never kicks in. You can see why here: #1008 (comment)

The HMR server needs to understand your application to properly bundle HMR events to the correct accept() handler. In dev, we scan files as we serve them. In build, we should scan them when we build them instead.

This ticket should be resolved by copying the dev HMR management logic into build.ts. There should be lots of reuse here, and the PR should refactor so that most of this logic can be shared between dev.ts and build.ts.

@truongsinh
Copy link

is never called and full HMR never kicks in. You can see why here: #1008 (comment)

is the link correct? it points to a thumb up emoji

@FredKSchott
Copy link
Owner Author

lol I meant to link to the line of code that the comment is referring to.

@ZYSzys
Copy link
Contributor

ZYSzys commented Dec 22, 2020

I've tried to do this while testing in react-typescript project, but I found the react-refresh plugin seems not to be work in build(production?) mode.

From this issue I saw this:

Don't forget that both Babel transform and this wrapping must only occur in development mode.

Looks like the react internal not support fast-refresh in production mode, correct me if I'm wrong.

@joehenry087
Copy link
Contributor

I am seriously considering taking a stab at this work. Any advice for me, or is anyone else currently on this? I'm pretty new to snowpack - I've spent some hours attempting to convert an existing express app to snowpack specifically to use HMR, and also played around with some demo apps. So I have some familiarity with the docs, but totally new to the codebase.

I've spent an hour looking at build.ts and dev.ts.

@PatrickBauer
Copy link

PatrickBauer commented Jan 17, 2021

I'd really love this feature. I tried switching from webpack to snowpack but have a special use-case where I'm developing modules for a pre-existing software, in this case FoundryVTT, a virtual tabletop system.

As this software uses a closed source server and only allows me to load existing scripts from folders (instead of from an external URL) I can't use the dev-server as the software will look in my dist folder which is simply empty at the time and refuses to load anything.

I got it working without any hitches using the build --watch mode, but as HMR in this case always reloads the whole page I can't really use it to speed up my development process.

@FredKSchott
Copy link
Owner Author

@joehenry087 would obviously love your help! The basic premise is to copy the behaviors from dev.ts related to HMR and managing the scanned import graph:

  • whenever a JS file is built, scan & resolve its imports and call hmrEngine.setEntry(filePath, resolvedImports, ...
  • copy over the updateOrBubble() function from dev into build.ts. Or, even better, a place that both could share.

The other thing to figure out is how to rewrite the imports that point to the HMR-updated modules.In dev.ts you can see how we transform some imports into ${imp}?${reqUrlHmrParam}.

@FredKSchott
Copy link
Owner Author

This is definitely tricky though, give it a try but no worries if it ends up being too much

@rizrmd
Copy link

rizrmd commented Feb 10, 2021

any progress on this?, I also would like to help.

@joehenry087
Copy link
Contributor

Spent a bunch of time looking at it and got busy with other things 😂

@joehenry087
Copy link
Contributor

@rizkyramadhan I'm beginning work on this now through tomorrow. I'll be on discord JoebiWanKanobi#5940

@FredKSchott
Copy link
Owner Author

#2707 ends up rewriting the build pipeline entirely in a way that gives us build HMR for free! I'd love your help testing the PR if anyone is able to check out the branch or run with npm install snowpack@next

@melissamcewen melissamcewen added this to Needs Triage in Triage Mar 11, 2021
@melissamcewen melissamcewen moved this from Needs Triage to Ready to Implement in Triage Mar 15, 2021
@melissamcewen melissamcewen moved this from Ready to Implement to Pull Request Open in Triage Mar 15, 2021
@FredKSchott FredKSchott moved this from Pull Request Open to Ready to Implement in Triage Mar 19, 2021
@trevyn
Copy link

trevyn commented Feb 11, 2022

Can confirm that snowpack build --watch --hmr --no-bundle is working for me.

I did notice that sometimes my server is still reading the old version of the file at the time it gets the refresh request. Not sure if this is something on my side, or if the updated files need to be fsync'ed or something before the refresh request is sent. Saving a second time picks it up, or I added a 50ms delay in my server between when it gets a request and when it reads the file in development mode, and that works too. 🤣

@FredKSchott FredKSchott removed this from Ready to Implement in Triage Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.1 contributors welcome! contributors welcome!
Projects
None yet
Development

No branches or pull requests

7 participants