hmr does not work for dependencies returned from svelte-preprocess

[dominikg]

Description

During svelte-preprocess, additional dependencies can be encountered and are returned as an array. Changes to these files currently do not trigger the expected hot module reload during vite dev

Reproduction

use one of the many ways to define an additional dependency by referencing a file in a svelte component that is then preprocessed. eg

<style lang="scss" src="./extra-file.scss"></style>

Additional Information

original bugreport on sveltekit: sveltejs/kit#849

Things to do

1. ensure the dependencies are watched correctly. For most files this should already be the case as they are inside the vite root. Call 'addWatchFile' anyways.
2. ensure that edits to these files trigger hmr update

• either by adding the files as dependencies to the respective js/css module of the .svelte file on the moduleGraph
• or by triggering an update event on the svelte file manually

[dominikg]

here's how vite's own css plugin does it: https://github.com/vitejs/vite/blob/2a89c57d3fb2dedbe595c4d49c454f3d138e6414/packages/vite/src/node/plugins/css.ts#L179

[dominikg]

Here's my take on updating the module graph https://github.com/sveltejs/vite-plugin-svelte/tree/watch-preprocess-deps

Unfortunately vites own importAnalysis plugin runs last and if i understand correctly whats happening it undoes my changes. :/

cc @rixo

[alesvaupotic]

For a change in Windi config file, CSS HMR happens correctly for direct classes, but not for @apply in components. For attached example, a change to windi-500 color triggered reload on text-windi-500 in class, but no change was made for @apply bg-windi-500 in component.

[dominikg]

thank you for mentioning the windicss case here.
This is a really tricky one because the windicss config file is not even an indirect dependency of the .svelte component but rather something that changes the output of the svelte preprocessor. Oh my. I believe vite-plugin-windicss already triggers a full reload if the windicss config changes but some cache (either inside vite or the one of vite-plugin-svelte) might prevent this reload from triggering a full recompile of all svelte components (or at least the ones actually containing windicss rules).

@kaisermann whats the take on config files that change preprocessor output, should they be added to preprocess dependencies output? Would this apply to postcss.config.js too?

[kaisermann]

@dominikg I'm not sure there's a bulletproof way to do this. We could do it for postcss.config.js and tsconfig.json, which are config files we need directly. However, a postcss plugin or a babel plugin, or anything really can request some file that we're not aware of. We could have a config to manually define some files as dependencies though.

[dominikg]

In an ideal world we would track all the config files that could affect preprocessor output, and recompile svelte components that were affected by one of the preprocessors and only trigger an update if the actual compiled output changed as a result of the config change.
But that would be really hard to get right and maintain. Imho editing a config file warrants a full reload, we just need to be aware that the file exists and was changed and nuke everything cached to prevent stale caches from delivering wrong results.

I guess there is a price to pay if you edit global config files and that price is a full reload.

[alesvaupotic]

Yes, that's why I took the https://github.com/antfu/vite-plugin-restart into my setup. It got some globbing issues on Windows, which I am ironing out currently, but it does the job. Thankfully Vite is so quick that this full reload is barely noticable. I understand it is not ideal, but I'll take a slower correct response anytime over no response.

Taking into account that config files rarely change, I suppose we can live with such workflow.

[rixo]

Tracking config files isn't worth it. There's so many possible cases -- all of them soon expanding to .mjs, .cjs, etc -- that would be a promise we cannot fully deliver, scoring at best something between deceive and disappoint, depending on user setup. Furthermore, it's something nobody really expects, in a cold path with no high returns even if it worked... And, all in all, far more easier to handle in userland with restart plugin + I know what config files I'm actually using in my project (even that seems too much trouble to me).

The initial point of the issue, tracking code dependencies, in the hot path, is what really matters IMO.

[dominikg]

As vites importAnalysis plugin prevents us from manipulating the module graph to add the dependencies here's an alternative approach: https://github.com/sveltejs/vite-plugin-svelte/tree/fix/watch-preprocessor-dependencies

preprocess dependencies are watched for changes and when they are changed, it triggers a change event on the parent svelte component file so standard vite handleHotUpdate mechanism runs.

vite-plugin-svelte/packages/vite-plugin-svelte/src/utils/preprocess.ts

Line 110 in 80d448e

dependants.forEach((d) => {

Tracking of dependencies/dependants is done via the compile cache

Initial tests look good: https://github.com/sveltejs/vite-plugin-svelte/blob/fix/watch-preprocessor-dependencies/packages/playground/svelte-preprocess/__tests__/svelte-preprocess.spec.ts

Thoughts?

[alesvaupotic]

How do I test this?

[dominikg]

Either wait until its released (hopefully soon, just tryin to tie up a few loose ends when files get deleted/added) or clone and build yourself locally. Find me on discord if you need help with it.

[alesvaupotic]

Thanks, no hurry. I'll wait for release.

[Kapsonfire-DE]

can confirm, with this branch it works like a charm!