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

[Bug?]: CSS styles in dev mode broken #1810

Open
2 tasks done
dominictobias opened this issue Feb 15, 2025 · 3 comments
Open
2 tasks done

[Bug?]: CSS styles in dev mode broken #1810

dominictobias opened this issue Feb 15, 2025 · 3 comments
Assignees
Labels
bug Something isn't working vinxi related to vinxi

Comments

@dominictobias
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

  • Cannot change e.g. the color of something in DevTools because styles are duplicated but somehow the duplicate is hidden
  • Broken for CSS and CSS Modules

Expected behavior 🤔

  • Should be able to change CSS properties in dev tools

Steps to reproduce 🕹

Steps:

  1. bun create solid
  2. Create Basic project
  3. Add color: red; to Counter.css
  4. Try to change it in DevTools - nothing happens (tested in latest Chrome and Firefox)
Screen.Recording.2025-02-16.at.00.23.51.mov

Chrome doesn't show the styles as duplicated but Firefox does:

Image
Image
Image

Context 🔦

No response

Your environment 🌎

"dependencies": {
    "@solidjs/meta": "^0.29.4",
    "@solidjs/router": "^0.15.3",
    "@solidjs/start": "^1.1.1",
    "solid-js": "^1.9.4",
    "vinxi": "^0.5.3"
  }
@dominictobias dominictobias added bug Something isn't working needs triage labels Feb 15, 2025
@katywings katywings self-assigned this Feb 15, 2025
@katywings
Copy link
Contributor

katywings commented Feb 15, 2025

Hi @dominictobias, thank you a lot for this detailed issue text with the screenshots and repro ❤️. I was also able to reproduce this with Chrome. Firefox on the other hand seems to be less picky when it comes to duplicate styles.

Ignore this :)

So regarding the problem: Vite Dev only picks up styles mounted under head, but we mount route-related styles (such as Counter.css under body. Vite will not detect them there, instead create duplicate styles during client hydration and place them in head. So we end up with two styles in the document. If you try moving the Counter.css import to the app.tsx you will find that the problem magically goes away 😅.

P.S. if all of this doesn't make a lot of sense to you, you can ignore it 😁, I mostly am writing it down here that it isn't forgotten.


With all of that said this issue definitely needs further analysis from our side. The questions that I still have on my mind at the moment are:

  1. Was this already a problem with Start 1.0 & Vite 5?
    1. ...or did Vite 6 change how styles are detected in the document?
  2. Is there a way to make Vite detect styles in the body?
    1. If not: would Vite be interested to add such a way?
    2. If still not: How can we render the route <style> tags under head?

I took another look at this, the issue actually lies in this line: https://github.com/nksaraf/vinxi/blob/e203a5b87ff63f26cc83a04ff7bc02640bc3fb46/packages/vinxi/runtime/style.js#L45

@katywings katywings added the vinxi related to vinxi label Feb 16, 2025
@dominictobias
Copy link
Author

dominictobias commented Feb 17, 2025

Not sure

Hi @dominictobias, thank you a lot for this detailed issue text with the screenshots and repro ❤️. I was also able to reproduce this with Chrome. Firefox on the other hand seems to be less picky when it comes to duplicate styles.

Ignore this :)

So regarding the problem: Vite Dev only picks up styles mounted under head, but we mount route-related styles (such as Counter.css under body. Vite will not detect them there, instead create duplicate styles during client hydration and place them in head. So we end up with two styles in the document. If you try moving the Counter.css import to the app.tsx you will find that the problem magically goes away 😅.
P.S. if all of this doesn't make a lot of sense to you, you can ignore it 😁, I mostly am writing it down here that it isn't forgotten.

With all of that said this issue definitely needs further analysis from our side. The questions that I still have on my mind at the moment are:

  1. Was this already a problem with Start 1.0 & Vite 5?

    1. ...or did Vite 6 change how styles are detected in the document?
  2. Is there a way to make Vite detect styles in the body?

    1. If not: would Vite be interested to add such a way?

    2. If still not: How can we render the route <style> tags under head?

      • Related code:

          [solid-start/packages/start/src/router/lazyRoute.ts](https://github.com/solidjs/solid-start/blob/9b6ba7f919b92ef381fdef0a1fdab8362657cb3f/packages/start/src/router/lazyRoute.ts#L55)
        
        
             Line 55
          in
          [9b6ba7f](/solidjs/solid-start/commit/9b6ba7f919b92ef381fdef0a1fdab8362657cb3f)
        
        
        
        
        
            
              
               return [...styles.map((asset: Asset) => renderAsset(asset)), createComponent(Component, props)];
        

I took another look at this, the issue actually lies in this line: https://github.com/nksaraf/vinxi/blob/e203a5b87ff63f26cc83a04ff7bc02640bc3fb46/packages/vinxi/runtime/style.js#L45

Comparing the lazyRoute of react vs solid, react looks like it's updating the style in the head instead of writing it into the body:

Solid - like what you posted:
https://github.com/nksaraf/vinxi/blob/e203a5b87ff63f26cc83a04ff7bc02640bc3fb46/packages/vinxi-solid/lazy-route.jsx

Image

React:
https://github.com/nksaraf/vinxi/blob/e203a5b87ff63f26cc83a04ff7bc02640bc3fb46/packages/vinxi-react/lazy-route.js

Image

Perhaps Solid can do that approach too?

@katywings
Copy link
Contributor

@dominictobias Yeah thats part of the solution I think 🙏, but ultimately the problem is: Chrome Devtools becomes flaky as soon as there are two <style> tags with the same content - even if both are rendered in the head. I just tried this before, if I manually add the bottom styles to <head> I am unable to change the background-color of .letsGo in Chrome Devtools 🙈.

<style>{`.letsGo { background-color: yellow; }`}</style>
<style>{`.letsGo { background-color: yellow; }`}</style>

So if we want to fix this, we have to deduplicate the <style> tags or hope that Chrome Devtools gets fixed 😬.

As part of #1743 we are currently discussing and laying out some new foundations together with Tanstack Start. I am including this issue in that discussion and hope we can include a fix for this, in the coming fs-router / assets handling solution. As soon as I know more, I'll try to keep you updated 🙂.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vinxi related to vinxi
Projects
None yet
Development

No branches or pull requests

2 participants