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

Incorrect classnames during SSR #937

Closed
2 tasks done
endigma opened this issue Dec 2, 2022 · 5 comments · Fixed by #952
Closed
2 tasks done

Incorrect classnames during SSR #937

endigma opened this issue Dec 2, 2022 · 5 comments · Fixed by #952

Comments

@endigma
Copy link
Contributor

endigma commented Dec 2, 2022

Describe the bug

During server rendering, the target div is rendered with class routes__1ry5ddd0, which has no corresponding styles. It hydrates to _1ry5ddd0, which has the correct styles.

The provided example is nothing but the SvelteKit starter (pnpm create svelte@latest) with the vite plugin and a basic style added. It can be easily observed by comparing the hydrated DOM with "view page source" etc. You can also observe the lack of styling using an addon like uBlock Origin or uMatrix or NoScript etc to block JS.

You can clone the repo, run pnpm install, pnpm dev to see the issue.

The most outwardly visible symptom is a FOUC, you may or may not be able to see it depending on your setup.

The reproduce conditions are a little complicated, so I've created a table to help. There may be other reproduce conditions I'm not aware of yet, but these work on the repo I provided.

Running Mode identifiers Reproduce?
vite dev "short" Yes
vite dev "debug" Yes
vite build && vite preview "short" Yes
vite build && vite preview "debug" No

Reproduction

https://github.com/endigma/vanilla-extract-ssr-bug

System Info

System:
    OS: Linux 6.0 Void
    CPU: (16) x64 AMD Ryzen 7 5800X 8-Core Processor
    Memory: 6.65 GB / 31.26 GB
    Container: Yes
    Shell: 3.5.1 - /usr/bin/fish
  Binaries:
    Node: 16.15.1 - ~/.asdf/shims/node
    Yarn: 1.22.18 - /usr/bin/yarn
    npm: 8.11.0 - ~/.asdf/shims/npm
  Browsers:
    Chromium: 107.0.5304.87
    Firefox: 107.0
  npmPackages:
    @vanilla-extract/css: ^1.9.2 => 1.9.2 
    @vanilla-extract/vite-plugin: ^3.7.0 => 3.7.0 
    vite: ^3.2.4 => 3.2.4

Used Package Manager

pnpm

Logs

No response

Validations

@endigma endigma changed the title Incorrect classnames during SSR with short identifiers Incorrect classnames during SSR Dec 3, 2022
@AlexandrLesiv
Copy link

AlexandrLesiv commented Dec 4, 2022

I faced the same issue. Appears that the name of generated class names in a node environment depends on the NODE_ENV environment variable. So, if NODE_ENV === 'production' then it will generate class names like "_1ry5ddd0", and if NODE_ENV === 'development' then it will generate class names like "routes__1ry5ddd0". This is undocumented behavior. I think that it should be added to the documentation.

Btw, I resolved the issue by setting the correct NODE_ENV when needed

@endigma
Copy link
Contributor Author

endigma commented Dec 4, 2022

The differing NODE_ENV doesn't explain why it's different on the server vs client, in addition to being totally wrong on the server. routes_abc has no styles associated with it, which causes a FOUC before it hydrates to _abc. The fix would be picking consistent class names on the server and client.

There is a config option for classname format, so I do not understand why NODE_ENV affects it at all? If I wanted NODE_ENV to determine which format to use I would write that in the configuration.

@askoufis
Copy link
Contributor

askoufis commented Dec 5, 2022

Did a bit of debugging in your reproduction and found that adding vite-plugin-svelte to the list of plugins to force emit SSR (the qwik plugin needed the same treatment) fixes the issue. However I'm unsure if this is the correct solution as I'm unfamiliar with this part of the codebase.

@endigma
Copy link
Contributor Author

endigma commented Dec 5, 2022

SvelteKit is pretty similar to the other frameworks mentioned, perhaps this forceEmitCssInSsrBuild setting should be user configurable? Maintaining an in-library list of exceptions here seems weird, just adding some docs about it and exposing as an option seems far more robust to me. If it was set up to take bool | () => bool then the existing code could be used as the default value.

@askoufis
Copy link
Contributor

askoufis commented Dec 6, 2022

Making this user-configurable has been discussed before, so it's likely worth taking another look at it. I'm also curious as to whether #851 will have an affect on this issue, if it ends up getting merged.

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

Successfully merging a pull request may close this issue.

3 participants