Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HMR not working for CSS Modules #1127

Closed
brillout opened this issue Sep 18, 2023 · 10 comments
Closed

HMR not working for CSS Modules #1127

brillout opened this issue Sep 18, 2023 · 10 comments
Labels

Comments

@brillout
Copy link
Member

brillout commented Sep 18, 2023

Reproduction: https:/brillout/vite-plugin-ssr-mvp.

(Failed reproduction using vanilla Vite with SSR: https:/brillout/vite-bug-hmr-css-modules — it fails to reproduce because it doens't have any *.module.css?direct module.)

(Vanilla Vite without SSR working example: #261 (comment).)

@brillout
Copy link
Member Author

It seems like the problem is that Vite isn't setting *.module.css?direct modules to be an HMR boundary (i.e. isSelfAccepting), therefore triggering a full reload.

> npm run server:dev


> server:dev
> node ./server

Server running at http://localhost:3000
4:59:32 PM [vite-plugin-ssr][request(1)] HTTP request: /
4:59:33 PM [vite-plugin-ssr][request(1)] HTTP response / 200
node.id /home/rom/tmp/vite-plugin-ssr-mvp/pages/index/test.module.css
node.isSelfAccepting false
node.id /home/rom/tmp/vite-plugin-ssr-mvp/pages/index/index.page.jsx
node.isSelfAccepting true
mod.id /home/rom/tmp/vite-plugin-ssr-mvp/pages/index/test.module.css
hasDeadEnd false
node.id /home/rom/tmp/vite-plugin-ssr-mvp/pages/index/test.module.css?direct
node.isSelfAccepting false
No importer -> no HMR boundary found
mod.id /home/rom/tmp/vite-plugin-ssr-mvp/pages/index/test.module.css?direct
hasDeadEnd true
needFullReload true
5:00:08 PM [vite] page reload pages/index/test.module.css
5:00:08 PM [vite-plugin-ssr][request(2)] HTTP request: /
5:00:08 PM [vite-plugin-ssr][request(2)] HTTP response / 200
node.id /home/rom/tmp/vite-plugin-ssr-mvp/renderer/PageShell.css
node.isSelfAccepting true
mod.id /home/rom/tmp/vite-plugin-ssr-mvp/renderer/PageShell.css
hasDeadEnd false
node.id /home/rom/tmp/vite-plugin-ssr-mvp/renderer/PageShell.css?direct
node.isSelfAccepting true
mod.id /home/rom/tmp/vite-plugin-ssr-mvp/renderer/PageShell.css?direct
hasDeadEnd false
needFullReload false
5:00:24 PM [vite] hmr update /renderer/PageShell.css, /renderer/PageShell.css?direct

The interesting logs showing what I believe to be the issue:

node.id /home/rom/tmp/vite-plugin-ssr-mvp/pages/index/test.module.css?direct
node.isSelfAccepting false
...
node.id /home/rom/tmp/vite-plugin-ssr-mvp/renderer/PageShell.css?direct
node.isSelfAccepting true

Closing as it seems to be a Vite bug.

@wxin627 If you're up for it then dig into this file, I suspect the issue to be somewhere in there.

@brillout brillout closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2023
@wxin627
Copy link

wxin627 commented Sep 21, 2023

got it, thanks!

@brillout
Copy link
Member Author

Is vitejs/vite#16018 related?

@snake-py
Copy link

@brillout I think this is the same issue vitejs/vite#16074 and in the issue they say:

This is fixed by vitejs/vite#16018, but wanted to file it for tracking purposes since this seems like a pretty major issue that wasn't reported.

So, Yes I think it is related. I asked for clarification.

Do you think a possible workaround for this issue would be to completly disabling server rendering in dev mode? Would this be possible using vike?

@snake-py
Copy link

@brillout So it seems Vite has released a 2nd party plugin to fix the issue, https:/privatenumber/vite-css-modules I am trying to get it to work with Vike, but so far no luck. Maybe you have a minute to investigate if this is something vike would need to incorporate at a deeper level? Also see this comment.

@hi-ogawa
Copy link

I got a notification on Vite PR comment, so I'm checking in here.
As brillout wrote in #1127 (comment), this is due to non-self-accepting css module and the specific usage of ?direct. I've seen a same issue on my framework and also saw sveltekit has the same issue sveltejs/kit#10905 (though they use ?inline instead of ?direct).

I'm not sure if it's fair to say Vite's bug, but it's certainly a Vite SSR limitation where frameworks need to resort to ?xxx to get CSS on server side.

The workaround I experimented on my framework hi-ogawa/vite-plugins#346 is to filter out ?direct module from handleHotUpdate hook. It would look like this:

const config: UserConfig = {
  plugins: [
    {
      name: "hack-css-module-hmr",
      handleHotUpdate(ctx) {
        // prevent full reload due to non self-accepting css module.
        // here only "?direct" module should be filtered out as it doesn't have a parent module.
        if (ctx.file.includes("module.css")) {
          return ctx.modules.filter((m) => !m.id?.includes("?direct"))
        }
      },
    },
    react(),
    vike(),
  ]
}

@snake-py
Copy link

snake-py commented Aug 17, 2024

@hi-ogawa Thank you for checking in here. I tested the code snippet here https:/snake-py/vike-css-module-hmr and it works. @brillout maybe this would be something we can put into Vike?

const fileEndings = [....];
    {
      name: "hack-css-module-hmr",
      handleHotUpdate(ctx) {
        // prevent full reload due to non self-accepting css module.
        // here only "?direct" module should be filtered out as it doesn't have a parent module.
        if (ctx.file.includes("module") && fileEndings.some((ending)=>ctx.file.endsWith(ending)) ) {
          return ctx.modules.filter((m) => !m.id?.includes("?direct"))
        }
      },
    },

I`d be willing to help out with the implementation of this!

@brillout
Copy link
Member Author

@hi-ogawa Thank you for chiming in.

@snake-py Done and pre-released as 0.4.188-commit-373ae21.

@snake-py
Copy link

@brillout I tested the new tag here https:/snake-py/vike-css-module-hmr and it does not work, anything I am doing wrong here?

@brillout
Copy link
Member Author

"vike": "0.4.188-commit-373ae21" => remove the ^. (Btw. I ain't forgetting your eject PR, I'll get back to it once I'm done with my current priorities.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants