sveltejs / vite-plugin-svelte

Svelte plugin for http://vitejs.dev/
MIT License
864 stars 105 forks source link

Import with compilerOptions breaks HMR #577

Closed oskar-koli closed 1 month ago

oskar-koli commented 1 year ago

Describe the bug

I have a bit of an unusual use case, where I need to have access to both DOM and SSR compiled versions of a Svelte component in the frontend. I achieved this without a problem by using the "compilerOptions" import query option as described in advanced uses: import AppSSR from './App.svelte?svelte&direct&type=script&compilerOptions={"generate":"ssr"}';

But this is causing some weird behavior with HMR. I'm assuming vite-plugin-svelte only supports HMR for normal dom compiled components, which is fine, but it seems to me that the JSON in the compilerOptions param breaks any possibility of implementing the HMR yourself. I believe the problem is that &compilerOptions={"generate":"ssr"} is sometimes converted into &compilerOptions={%22generate%22:%22ssr%22}, which breaks HMR because there is no module accepting HMR events for this corrupted path and this results in hard refreshes. So it seems to me that the import paths are URL encoded at some point, but then not appropriately decoded.

I'm unsure if this encoding is happening because of this plugin or because of how Vite handles import paths. I can investigate this further, but felt like it might be a good idea to create an issue first.

There is also of course the possibility that the problem is not in Vite or this plugin but that I've just misunderstood something in how to set up the HMR boundary, in which case sorry for the spam and do go ahead and close this issue :)

Reproduction URL

https://stackblitz.com/edit/vitejs-vite-5npyoj?file=src/main.js

Reproduction

1) Open the Stackblitz repro, let it start up 2) If you make changes to App.svelte, the HMR works as expected. 3) Refresh the iframe inside Stackblitz (the built in browser, not the real browser) 4) If you now make edits to App.svelte, the HMR no longer works and does hard refreshes instead 5) If you look at the main.js through the Chrome dev tools, you can see that the import statement has been replaced with the corrupted version:

Screenshot 2023-01-16 at 9 38 21

Logs

No response

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    @sveltejs/vite-plugin-svelte: ^2.0.2 => 2.0.2 
    svelte: ^3.55.0 => 3.55.1 
    vite: ^4.0.4 => 4.0.4
dominikg commented 1 year ago

query params getting encoded after a refresh sounds like a generic vite problem. Are you able to reproduce it in a vanilla vite project with npm create vite@latest --template vanilla cc @bluwy

Apart from that i'd be interesting if you elaborated on your usecase a bit. These advanced queries are still experimental, very new and not meant for general use. (see remark at the top of the docs). Would love to learn more. Maybe open a thread on https://svelte.dev/chat in help > svelte-and-kit and tag me there.

Also cc @rixo, having a self-accepting module also accept updates of direct children sounds like something up his alley.

oskar-koli commented 1 year ago

I was able to reproduce the issue in a vanilla Vite project: https://stackblitz.com/edit/vitejs-vite-e32rng?file=counter.js,main.js

Steps:

  1. Make a change in "counter.js", HMR works fine
  2. Make a change in "main.js", HMR works fine
  3. Make a change in "counter.js", HMR no longer works and a hard refresh happens.

Looking at the source code in Chrome dev tools shows that after the change in main.js, the import statement of counter.js has been corrupted:

Screenshot 2023-01-19 at 14 17 05
dominikg commented 1 year ago

It was a bit confusing that counter isn't mounted in the example but i see the behavior. simply using the encoded form from for json isn't a workaround either as it would lead to double encoding for the % import * as counter from '/counter.js?t=1674143945289&json={%2522a%2522:%2522b%2522}';

Can you report this to vite with a link to here?

dominikg commented 1 month ago

did you ever get around to reporting this in vite? vps can't really do something about it

oskar-koli commented 1 month ago

I moved on from the side project I discovered this issue in, so I unfortunately never did get around to reporting it to Vite. It's alright to close this issue for my part.

dominikg commented 1 month ago

closing here, if you ever need this from vite, please go ahead and file.