Add svelte-hmr

[rixo]

Hey!

How would you feel about adding svelte-hmr in this plugin?

This PR is the gist of it.

This can be seen in action in this branch of my template.

The main point of contention is handling emitCss: true... svelte-hmr doesn't play very well with this currently, but it could probably be adapted to just change the CSS classes of existing elements, and rely on another CSS plugin for hot injection of CSS. Unfortunately, my hot fork of the postcss plugin doesn't to play along very well either...

In the end, maybe just forcing emitCss: false when hot: true would be sufficient? Still one hard point here would be that, ideally, the bundle.css (or so) normally produced by the user config would need to be overwritten with a blank file; both to avoid unsavory 404 on the missing CSS files in dev, and to avoid loading CSS from a previous build that would risk interfering with the HMR loaded one. This could probably be achieved by emitting empty files from the plugin, instead of the actual CSS.

Also, I've included a Nollup compat plugin that is automatically injected in the Rollup config when Nollup is detected (through an env variable it sets). Given there's only 2 HMR options for Rollup (to my knowledge), and Nollup has a real added value in dev with its speed, I'm keen to support it transparently to the user if possible. This compat plugin is injected automatically both by rollup-plugin-hot and rollup-plugin-svelte-hot, since people might want to use HMR only with Nollup, or Nollup in a non Svelte project.

Happy to discuss any point... Starting this PR to get the ball rolling!

[benmccann]

Most of the effort for faster reloading these days is focused on SvelteKit. I think we'd really like to consolidate the community around that. This PR looks fairly clean and reasonable, but I think as much as possible we'd like to just create one primary way of doing things that's really well supported (i.e. Snowpack via SvelteKit) and avoid splitting the relatively small community of users amongst different solutions, so that would be my primary hesitation with this

I'm also a bit curious if this really needs to be part of rollup-plugin-svelte or could be a separate Rollup plugin

[rixo]

Well, I really think it needs to be done.

My hot forks were only intended as an interim solution while dev was strangled here, but now that things are moving here it's becoming a maintenance burden for no benefit.

The cost here is very low, we're talking a few kb of JS with only one external dep that is already in this project. The option is off by default and, as you can see in the PR, the risk of leaking into other features while off is very low.

There are already a whole lot of Rollup projects out there because it was the recommended setup until... Some time in the future. Nollup + HMR can be a real benefit for those projects, if only they were told about it.

Furthermore HMR has been promised as a top priority on Svelte 3 FAQ since forever. If we consider svelte-hmr as a polyfill for a compiler feature, it only makes sense that the official Rollup plugin exposes this feature.

I don't see the point of making this yet another experimental community plugin to wrap around the official one when this can be a simple on/off toggle in the official plugin at a very low cost... It wouldn't do any good in the way of clearing confusion around HMR status in Svelte. And it would be kind of unfair to existing users who have followed official guidelines if we keep that a Snowpack / Kit only feature for no good reason.

[pngwn]

I think we need to be careful about svelte kit, while we want that to be the 'go-to' way for building apps, that doesn't mean we should purposefully hamstring other integrations.

I cant see a good reason why we wouldn't want this in, if the only way to make svelte-kit a success is to limit other tools then it isn't succeeding on its own merits. Rixo's core hmr solution is pretty agnostic to the tooling that utilises it for this reason, where the effort is low we should be aiming for parity.

There is also the issue that svelte kit doesn't exist yet in any meaningful sense, and even when it does not everyone will want to migrate plain svelte apps across.

[rixo]

Anything I can do to help move this forward? What would be missing?

[benmccann]

I don't understand it well enough to have an opinion on it yet. We'd probably need some docs on this if we were going to merge it about what it does, how it works, etc. Those details might help me get more comfortable with it. One of the things I'm most hesitant about is the nollup specific stuff. It's pretty small, so maybe that's okay, but I do wonder if there some way we can support Nollup without having to put code specifically for it in this code base

[rixo]

What it does and how it works is explained there.

Options that would be added to this plugin via the "hot" options are listed just above.

Is that the kind of thing you had in mind? Feel free if you have any question...

I can take a stab at summarizing a little bit of it here (i.e. in the PR). To be honest, I'm striving to make HMR a feature that just works and require no explanations or configuration for the end-user. So I think more than a short paragraph and a link back to svelte-hmr README, for people who really want to know more, might just be dead weight here... I'm happy to do however you think is more appropriate, though.

I get your point about Nollup support from this plugin. I think it made a lot of sense for the "hot" version but, here, it's more fat fetched. I'm gonna study alternatives (see if Nollup wants to take it, ask Nollup users to drop it themselves in their config, or maybe keep rollup-plugin-svelte-hot as a thin wrapper around this plugin instead of a full fledged fork...). Anyway, I think it's better to skip the question for now, so I'm going to update the PR to remove Nollup specific stuff.

[dummdidumm]

I think @Conduitry had some concerns about the HMR thing having too much knowledge of the Svelte internals which is brittle. So it might be good to flesh out an "official" HMR API in Svelte core.

[rixo]

I think @Conduitry had some concerns about the HMR thing having too much knowledge of the Svelte internals which is brittle.

That's something that I, at least, have been saying, since always.

So it might be good to flesh out an "official" HMR API in Svelte core.

Sure. Easier said than done, unfortunately. Main blocker is that there is a tight coupling with some specific tool (bundler / no-bundler) that is hard to escape.

The essence of HMR support is adding HMR accept / dispose handlers to the generated code, which the compiler could happily and efficiently do on its side. Unfortunately, there are half a dozen, even more, different HMR API for those handlers out there... There's essentially no 2 tools with exactly the same HMR API.

The "native" format output of svelte-hmr conforms to the esm-hmr specs (compatible with Snowpack, Vite, and rollup-plugin-hot). This spec does seem to gather some level of following, and compatibility layers (for example for Nollup or Webpack) can be implemented over it, so maybe it could be justified to aim for that. Or maybe we could let the compiler do most of the work and offer agnostic hooks / accept adapters that each tool would need to fulfill.

Anyway, this would surely needs extensive reflection and discussion in a better place than here, a Svelte RFC probably.

---

Regarding this PR, I've removed Nollup specific stuff for now, and added some notes of docs.

I'm also having second thoughts about this, though... Indeed, I find the communication that could be done around this would probably be somewhat unsatisfactory: "this is an option that enables HMR support, but it will require extra work from you because Rollup doesn't support HMR, and we won't give you the precise instructions here because nothing's set at an official level currently". Hmm.

In particular, decoupling the HMR support layer from the Nollup's compatibility layer seems a bit problematic in this context, because Nollup probably remains the easier and faster solution for HMR in a Rollup setup. Still, I get that this compatibility layer might not have its place here.

In the end, making HMR support a dedicated plugin, to be used in addition to this one, could actually make sense, while the feature is community supported. We could be more liberal about what's included there (Nollup support, extensive documentation about the current HMR implementation...). And that would solve the API alignment / maintenance burden between rollup-plugin-svelte and rollup-plugin-svelte-hot that has arisen lately -- since the hot plugin would only focus on transforming the resulting compiled code, the official plugin could have the API it wants and we'd probably remain compatible in almost any case.

Unfortunately, this is currently not possible, because the hot plugin needs access to the code before and after compilation, as well as the whole result of the compiler (vars, etc.), and the compile options that were used...

Maybe we could implement some kind of postprocess hook support in this plugin, to allow for the existence of a dedicated hot plugin beside this one?

Maybe we can let this plugin make the HMR transform without documenting it for now, and maintain some community documentation about the whole subject in a better place?

Maybe the maintenance burden for the hot fork of the plugin is starting to pale, in comparison with the complexity of the other options? Can you tell if we foresee many other breaking changes in the near future, here?

I fail to see an obvious answer... Which maybe tells something in favor of my last lead...

[benmccann]

Thanks for the explanation about the esm-hmr spec. It gives me a lot of comfort knowing the implementation is compatible with Snowpack, Vite, etc. It looks like https://github.com/preactjs/wmr/issues/257 may also be moving in that direction.

It seems like the main issue is that HMR requires a dev server and Rollup doesn't come with one like Webpack does. Given that, perhaps that best thing to recommend to users is that they use Snowpack, Svite, SvelteKit, wmr, etc. if they want HMR support while building with Rollup. In that case, the question is how can we best support @FredKSchott's work on snowpack-plugin-svelte, @dominikg's work on Svite, etc. Would merging this here help those use cases at all or is it better to have those libraries use svelte-hmr directly? Or another option might be to make a second Rollup plugin that's called after this one

[dominikg]

Merging this would certainly help in case of vite2, but as long as there is any solution that enables implementing a 'rollup-plugin-svelte-hot' by augmenting rollup-plugin-svelte that would also be good.

The transform flow preprocess->compile->makeHot is pretty similar in rollup-plugin-svelte-hot, snowpack-plugin-svelte, vite-plugin-svelte and properbly also webpack-loader-svelte. There has to be a better way than X tools doing the same dance to enable hmr for svelte.

[rixo]

It has been brought to my attention (https://github.com/rixo/svelte-hmr/issues/24) that Vite 2 can use (some) Rollup plugins directly. After some experimentation, it does indeed turn out that Svelte HMR works in Vite 2 with rollup-plugin-svelte-hot (https://github.com/rixo/svelte-hmr/issues/24#issuecomment-764985448). I've also tried the branch of this PR, and it works too, HMR included, just needed to also add emitCss: false.

So this tips me back into thinking that svelte-hmr should be added into this plugin. Indeed, if rollup-plugin-hot and even Nollup can be considered somewhat garage solutions, Vite is undoubtedly pretty mainstream. So that makes at least one major consumer of this plugin that can directly benefit from HMR support without much setup fuss.

To answer Ben's question, merging this won't help in the case of snowpack-plugin-svelte however, because it is using the Svelte compiler directly (in dev), and so it has no choice but to also wire svelte-hmr itself.

The main open question that remains for me is how much of HMR should be documented here. There's little doubt adding HMR to Rollup or using Nollup would deserve some sort of guide, but I still don't believe this plugin's README is the place for that. I guess the best solution would be to write these guides somewhere else (where? another markdown file in this repo? an article somewhere? any idea?) and link them from the README.

Anyway, I don't think the docs (or lack of docs) question is enough to block publishing the feature, so I'm marking this PR as ready for review.

For now, I've tried to pack the essential informations about HMR in the README, all while trying to avoid being too verbose (not sure how much I succeeded in that).

Also, I've changed the bit of code that was forcefully enabling dev mode when hot was enabled to the reverse, disabling hot if dev is off. I had copied this behaviour from the hot fork, where it may have made sense, because it was all about being hot... But not so much here. HMR won't work without dev mode but, anyway, I believe people's intention when they turn off dev mode is to produce a clean build. They surely don't want (broken) HMR handlers polluting their prod build, and requiring them to disable both dev and hot seems redundant.

[dominikg]

@rixo @benmccann is this still something worth pursuing?

[benmccann]

I missed the last maintainer's meeting, but believe there was some discussion about making svelte-hmr official. Whether that means moving it into this repo, into the org, etc. I don't know since I wasn't present. Happy to try to help things along if I can

[rixo]

I personally don't think pursuing official HMR support for Rollup is worth it anymore. I don't see a bright future with it now that Vite, Snowpack, etc are here. At the time, I thought this plugin would be reused in Vite but now we have a dedicated one. In my view Rollup HMR is more a legacy support thing now, for which my community fork probably is enough.