QwikDev / qwik

Instant-loading web apps, without effort
https://qwik.dev
MIT License
20.88k stars 1.31k forks source link

[🐞] useStylesScoped$ doesn't support nested CSS #6011

Open netzpixel opened 8 months ago

netzpixel commented 8 months ago

Which component is affected?

Qwik Runtime

Describe the bug

I have a simple qwik project with qwik city. I use scoped styles with useStylesScoped$. I'm styling the Link component via the :global() selector. In development all is fine. After deployment I noticed all the :global() styles are missing. I use the node deployment with the entry.server.js.

I investigated a bit and found out that if I start the app via the terminal with yarn start (after a successful build) all is fine, but if I start the same build via a node command (which I have to do for my specific deployment environment) the :global() styles are all gone.

I have a entry.cjs file to then just import the entry.express.js Like so:

(() => import('./entry.express.js'))()

With that setup (local and on the hosting provider) I get the same broken result.

Reproduction

https://github.com/netzpixel/qwik-broken-global-example

Steps to reproduce

npm install´ npm run dev` open the page

check the color of the first link (Docs) -> should be red


stop the running process node entry.cjs open the page

check the color of the first link (Docs) -> it's not red anymore

System Info

System:
    OS: macOS 14.3.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 136.67 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.11.0 - ~/.nvm/versions/node/v20.11.0/bin/node
    Yarn: 1.22.21 - ~/.nvm/versions/node/v20.11.0/bin/yarn
    npm: 10.4.0 - ~/.nvm/versions/node/v20.11.0/bin/npm
    bun: 1.0.7 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 122.1.63.165
    Chrome: 122.0.6261.128
    Edge: 122.0.2365.80
    Safari: 17.3.1
  npmPackages:
    @builder.io/qwik: ^1.5.1 => 1.5.1 
    @builder.io/qwik-city: ^1.5.1 => 1.5.1 
    undici: * => 6.8.0 
    vite: ^5.1.4 => 5.1.6

Additional Information

The system seams to be unrelated. The same happens on a linux server.

wmertens commented 8 months ago

Ok so the problem seems to be nested CSS not being supported by the scoped styles.

The original CSS (in a .module.css) is

ul {
  color: green;

  > :global(a) {
    color: red;
  }
}

During dev, the resulting CSS is

ul.⭐️d907jw-0 {
  color: green;

  > a {
    color: red;
  }
}

Note that this is nested CSS, and that Chrome supports this, but the scoped styles don't support it.

And in prod, it becomes

            ul.⭐️d907jw-0 {
                color: green
            }

            ul.⭐️d907jw-0>a.⭐️d907jw-0 {
                color: red
            }

Here, the nested rules are flattened, and the scoped styles correctly add the scoped classes.

So, it's a bug, but only because it should also not be red in dev mode.

To get the Link to be red, you have to do const {scopeId} = useScopedStyles$(...) and then <Link class={scopeId} />. This is the correct way to pass scopes down to Qwik Components.

netzpixel commented 8 months ago

This is in conflict with the docs: https://qwik.dev/docs/components/styles/#global-selector The <Link class={something} /> is only for non scoped css according to the docs. Using the ?inline parameter on the style file import changes how the file is read (I'm assuming here) and therefor I can't use it in the same way.

So what is the correct way to handle scoped css? And even better would be to know how to use scoped scss (but we can ignore that for the moment if it makes things way to complicated).

wmertens commented 8 months ago

TBH I think the best way to style is using Tailwind (or UnoCSS).

And to style Qwik components it's best to use the scopeId.

But the behavior of global is a bug

netzpixel commented 8 months ago

Ok, I finally found a solution. The hint with the nested stuff was essential:

Instead of:

ul {
  color: green;

  > :global(a) {
    color: red;
  }
}

I have to write this:

ul {
  color: green;
}

ul  > :global(a) {
  color: red;
}

I think the qwikest way to improve the issue would be to update the docs to remove the nesting there.

But still, the bug should be fixed.

netzpixel commented 8 months ago

Ok, I think I have to revert my last comment... It worked in one scenario and doesn't in another. It works if the global is added to an element which already is tracked. Example:

this works:

nav:global(.open) {
  display: block;
}

this doesn't:

nav :global(a) {
  display: block;
}

Reason seams to be that the first example only adds the .open via JS to an element which has a scoped class already and the second doesn't. The first one results in something like this: nav.open.⭐️rwl3jt-0 { while the second one creates something like this: nav.⭐️rwl3jt-0 a.⭐️rwl3jt-0 { which doesn't work since the .⭐️rwl3jt-0 doesn't exist on the a tag which still comes from the Link component.

wmertens commented 8 months ago

Ideally, someone makes a PR that fixes nested global handling in dev

RaiVaibhav commented 8 months ago

After reading the doc, I want to say thank you everything.

I am highly interested in solving the problem/issue, but I need help in understanding.

brandonpittman commented 1 month ago

So this seems to be the cause of #6982.

My problem is that Qwik is un-nesting @starting-style. If it left it in place, this issue wouldn't happen.

wmertens commented 1 month ago

@brandonpittman only 87% of browsers supports nested CSS right now, so IMHO the correct behavior is to unnest, but then correctly.

brandonpittman commented 1 month ago

@wmertens This feature is newer than even nesting, but it's a progressive enhancement. If it doesn't work, I don't mind.

Qwik is doing too much here. This un-nesting behavior should be left to something like PostCSS.

But even if it un-nested it, if Qwik appended the scope ID to the stuff that it nested, things would still work.

wmertens commented 1 month ago

Unfortunately the CSS can't be processed by post css, the optimizer can't do it.

But indeed it's a bug and it should add the scope correctly.

wmertens commented 1 month ago

...or indeed leave it alone

brandonpittman commented 1 month ago

...or indeed leave it alone

Seems like the easiest solution.