sveltejs / kit

web development, streamlined
https://svelte.dev/docs/kit
MIT License
18.77k stars 1.96k forks source link

HMR not working with imported CSS (Module) files #10905

Open aradalvand opened 1 year ago

aradalvand commented 1 year ago

Describe the bug

Editing CSS Module files that are imported in .svelte components causes a full page reload, while hot-reloading is expected (and demonstrably possible, but more on that in the "Additional information" section).

Before somebody asks "why aren't you using the <style> tag directly in your Svelte components and taking advantage of Svelte's built-in CSS scoping?", because they are not good enough, and I'm not alone in thinking that.

Reproduction

  1. Clone https://github.com/aradalvand/sveltekit-hmr-css-test
  2. Run pnpm install
  3. Run pnpm run dev
  4. Go to the src/lib/Button.module.css file, and change the background-color to blue.
  5. Save the file.
  6. Notice how a full page reload is triggered in the browser

Logs

No response

System Info

System:
    OS: Linux 5.15 Ubuntu 22.04.2 LTS 22.04.2 LTS (Jammy Jellyfish)
    CPU: (4) x64 Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz
    Memory: 7.21 GB / 11.63 GB
    Container: Yes
    Shell: 3.6.1 - /home/linuxbrew/.linuxbrew/bin/fish
  Binaries:
    Node: 20.6.0 - /home/linuxbrew/.linuxbrew/bin/node
    npm: 9.8.1 - /home/linuxbrew/.linuxbrew/bin/npm
    pnpm: 8.7.1 - /home/linuxbrew/.linuxbrew/bin/pnpm
  npmPackages:
    @sveltejs/adapter-auto: ^2.0.0 => 2.1.0
    @sveltejs/kit: ^1.20.4 => 1.26.0
    svelte: ^4.0.5 => 4.2.2
    vite: ^4.4.2 => 4.5.0

Severity

annoyance

Additional Information

  1. Put the following code in the <script> section of your root +layout.svelte file (or, in the repro app linked above, it could be in the +page.svelte file):
    if (import.meta.hot) {
    import.meta.hot.on('vite:beforeFullReload', () => {
        throw '(skipping full reload)';
    });
    }
  2. Make a change to the Button.module.css file again (change the background-color back to red), and save the file.
  3. At this point, if you look at the browser console, you'll see Uncaught (in promise) (skipping full reload), as expected; the full page reload is prevented.
  4. Now, make a meaningful change (not just whitespace, change the button's label for example) to the Button.svelte file, and then save the file.
  5. Back to the browser, you can see that the new CSS change has been hot-reloaded as well. I think this showcases that HMR for these CSS files is perfectly possible, so this has to be a matter of misconfiguration or something similar on the part of SvelteKit.

Let me know. Thanks.

john-michael-murphy commented 11 months ago

I'm also encountering this issue with standard css fiels. We (unfortunately) have to edit global css files in our SvelteKit project and the full reloads are a killer. It appears as if SvelteKit hot reloads first, and then follows with a full reload. Is that expected behavior?

wighawag commented 10 months ago

I got the same issue as you @john-michael-murphy

I import my global css in my +layout.svelte file and whenever I make change to the css the page reload. I expect svelte+kit+vite to be able to handle hot reload by swapping css

ekhaled commented 5 months ago

We have this problem as we use a global scss file.

I monkey patched kit, this line: https://github.com/sveltejs/kit/blob/@sveltejs/kit@2.5.10/packages/kit/src/exports/vite/dev/index.js#L205

- !(query.has('raw') || query.has('url') || query.has('inline'))
+ !(query.has('raw') || query.has('url') || query.has('inline') || query.has('noinline'))

and then in my +layout.svelte, the following change:

- import "../scss/app.scss";
+ import "../scss/app.scss?noinline";

TL;DR: I made sveltekit ignore inlining the css when that particular search param noinline exists.

Seems to be working well.

wighawag commented 5 months ago

Thanks @ekhaled ! I can confirm it works.

Is that something we should merge in somehow ?

ekhaled commented 5 months ago

Thanks @ekhaled ! I can confirm it works.

Is that something we should merge in somehow ?

Feel free to create a PR. I'll see if I can create one when I have a bit of time in hand.

@dummdidumm do you think this is something that might get accepted into core?

I'm sure a lot of people use global css with sveltekit. And this does increase productivity a lot when building site templates etc.

lolcabanon commented 3 months ago

We have this problem as we use a global scss file.

I monkey patched kit, this line: https://github.com/sveltejs/kit/blob/@sveltejs/kit@2.5.10/packages/kit/src/exports/vite/dev/index.js#L205

- !(query.has('raw') || query.has('url') || query.has('inline'))
+ !(query.has('raw') || query.has('url') || query.has('inline') || query.has('noinline'))

and then in my +layout.svelte, the following change:

- import "../scss/app.scss";
+ import "../scss/app.scss?noinline";

TL;DR: I made sveltekit ignore inlining the css when that particular search param noinline exists.

Seems to be working well.

Wow this is sooooo cool!! Still working with @sveltejs/kit v2.5.20, svelte v4.2.18 and vite v5.3.5.

No PR yet? I am not familiar with the Svelte/Kit codebase, so I would be afraid to (move fast and) break things in unexected ways, but I'm very excited to see it seems to be just around the corner!

ekhaled commented 3 months ago

There is this library called patch-package which can be used to to automate the patching on postinstall.

Your would end up with something like this structure:

Screenshot 2024-08-08 at 14 09 21

I'm also including the patch file, I'm sure it will help. The name of this patch file matters. Please read up on the patch-package docs.

@sveltejs+kit+2.5.18.dev.patch

lolcabanon commented 3 months ago

There is this library called patch-package which can be used to to automate the patching on postinstall.

Awesome thanks! I was trying to setup something similar! :)

lolcabanon commented 3 months ago

Just found out about pnpm patch command in the patch-package readme.

  1. Run pnpm patch <package> :

    
    $ pnpm patch @sveltejs/kit
    Patch: You can now edit the package at:
    
    /tmp/e86e895e5b7d2b3e21ca36d5c57bdc19

To commit your changes, run:

pnpm patch-commit '/tmp/e86e895e5b7d2b3e21ca36d5c57bdc19'


2. Edit relevant file (here `src/exports/vite/dev/index.js`) and save
3. Commit patch :
```bash
$ pnpm patch-commit '/tmp/e86e895e5b7d2b3e21ca36d5c57bdc19'

This gets added to package.json :

{
        "...": "...",
    "pnpm": {
        "patchedDependencies": {
            "@sveltejs/kit@2.5.20": "patches/@sveltejs__kit@2.5.20.patch"
        }
    }
}

IDK yet about maintenance and updates but it look great for now!

Edit : Ok, looks like it's not that cool yet when updating deps.

Edit 2 : Ok, I read the thread to the end, and they just released pnpm 9.7.0 which work on the update friction.