Skip to content

fix: process style modules at the end during HMR - #1257

Closed
ocavue wants to merge 1 commit into
sveltejs:mainfrom
ocavue-forks:ocavue/js-first
Closed

ocavue wants to merge 1 commit into
sveltejs:mainfrom
ocavue-forks:ocavue/js-first

Conversation

@ocavue

@ocavue ocavue commented Jan 7, 2026 •

Copy link
Copy Markdown
Contributor

During the development of withastro/astro#14821, I noticed that sometimes HMR doesn't work. After some debugging, here is the reason in my opinion:

  1. When the <style> tag in a .svelte file changes, hotUpdate.handler is called. ctx.modules contains two modules: Counter.svelte?svelte&type=style&lang.css and Counter.svelte. Notice the order here.
  2. await transformRequest(mod.url) is called first for Counter.svelte?svelte&type=style&lang.css.
  3. The Vite plugin loadCompiledCss's load.handler method is called, and a cached CSS is returned.
  4. Since the CSS for Counter.svelte?svelte&type=style&lang.css is from the previous cache, the module Counter.svelte?svelte&type=style&lang.css is marked as not affected.
  5. await transformRequest(mod.url) is called for Counter.svelte.
  6. The Vite plugin compile's transform.handler method is called. In this step, the .svelte file is actually handled by the Svelte compiler, and the cached CSS is updated.
  7. Since the JS part of this .svelte file doesn't change, the module Counter.svelte is marked as not affected too.
  8. Both modules are not affected, thus HMR is not triggered.

This PR fixes the issue by processing Counter.svelte first, which triggers the Svelte compiler and updates the cached CSS. Only after that, it processes Counter.svelte?svelte&type=style&lang.css.


I don't know how to test this patch in this pull request, though. While running hmr.spec.ts, the ctx.modules are in the order [HmrTest.svelte, HmrTest.svelte?svelte&type=style&lang.css]. This order won't cause the HMR issue I described above. I suspect that the different order of ctx.modules has something to do with the fact that astro is an SSR framework.

if (svelteRequest) {
const { modules } = ctx;
const svelteModules = [];
const svelteJsModules = [];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if svelteJsModules is a good variable name or not.

@dominikg dominikg Jan 7, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its ok. technically could be a little confusing with .svelte.js files but they are not processed by the hot-update plugin at all so its fine.

an alternative would be to simply sort svelteModules instead of throwing them into 2 different buckets first. the number of modules in here is super low, usually its exactly 2, so perf really isn't an issue

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an alternative would be to simply sort svelteModules

I feel that the following code is a bit hard to read.

svelteModules.sort((a, b) => {
    const numA = SVELTE_VIRTUAL_STYLE_ID_REGEX.test(a.id || '') ? 2 : 1;
    const numB = SVELTE_VIRTUAL_STYLE_ID_REGEX.test(b.id || '') ? 2 : 1;
    return numA - numB
})

@dominikg dominikg Jan 7, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need a custom comparator in this case, the css module id is constructed with a suffix, so svelteModules.sort() ensures that Foo.svelte?... comes after Foo.svelte.

edit: do'h, you need to map by id

svelteModules.sort((a,b)=> a.id > b.id ? 1 : a.id===b.id ? 0 : -1)

@ocavue
ocavue marked this pull request as ready for review January 7, 2026 12:03
@dominikg

dominikg commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

this is really strange. Due to the fact that the virutal style module is imported by the js module iit should always come afterwards in ctx.modules of hotUpdate.
Your analysis is correct if it doesn't so the fix makes sense but i'd still love to know what happened here. It would still fail if the parent component js module wasn't part of the list of modules at all, the only fix to that would be to allow css to be the first request and trigger a compile. ( see #1032 )

@dominikg

dominikg commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

regarding ssr tests. the kit-node e2e test is using ssr and also tests hmr, but the module order is the expected one there too.

@ocavue

ocavue commented Jan 7, 2026 •

Copy link
Copy Markdown
Contributor Author

but i'd still love to know what happened here

I created a repo for easier debugging of this issue: https://github.com/issueset/vite-plugin-svelte-hmr-issue.git

There are three files under components/. Update the <style> block in each of them, and you will see the following output in the terminal:

00:03:14 watching for file changes...
[DEBUG] ctx.modules: [
  '/src/components/client-load.svelte?svelte&type=style&lang.css',
  '/src/components/client-load.svelte'
]
00:03:20 [watch] src/components/client-load.svelte
[vite] program reload
[DEBUG] ctx.modules: [
  '/src/components/client-only.svelte',
  '/src/components/client-only.svelte?svelte&type=style&lang.css'
]
00:03:22 [watch] /src/components/client-only.svelte?svelte&type=style&lang.css
[DEBUG] ctx.modules: [
  '/src/components/ssr-only.svelte?svelte&type=style&lang.css'
]

Notice that for ssr-only.svelte, only the style module is shown, which is kind of makes sense as there is zero JS shipped to the client side. In this case, even my current patch that re-order svelteModules doesn't work.

@ocavue

ocavue commented Jan 7, 2026

Copy link
Copy Markdown
Contributor Author

the only fix to that would be to allow css to be the first request and trigger a compile

I submitted another PR to implement this behavior #1258

Based on my test, it works fine for all three cases shown in my previous comment

@dominikg

dominikg commented Jan 8, 2026

Copy link
Copy Markdown
Contributor

closing in favor of #1258

@dominikg dominikg closed this Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants