QwikDev / qwik

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

[šŸž] The priority of the `class` attribute is confusing. #6664

Closed genki closed 4 months ago

genki commented 4 months ago

Which component is affected?

Qwik Runtime

Describe the bug

I am writing components that the class attribute would be specified from manywhere. This is a simple example.

const Home = component$<PropsOf<"div">>((props) => {
  return (
    <div {...props} class="home">
      Home
    </div>
  );
});

export default component$(() => {
  return (
    <>
      <Home class="top" />
    </>
  );
});

There is a component that accept the class attribute, and setting another class by itself. In this case, I expected the class of the <div> element to be top home or home. But the real, the class attribute became to top. Is this the expected behaviour?

While digging the specification, I find the setClasslist function at https://github.com/QwikDev/qwik/blob/281de9701ecae0affcf18447985646917a2ef373/packages/qwik/src/core/render/dom/operations.ts#L129 but it is not referenced from anywhere.

Reproduction

https://stackblitz.com/edit/qwik-starter-gbb6ic?file=src%2Froutes%2Findex.tsx

Steps to reproduce

Please visit the link above and open the dev tools then check the class attribute of the Home component.

System Info

System:
    OS: macOS 14.5
    CPU: (8) arm64 Apple M2
    Memory: 133.39 MB / 24.00 GB
    Shell: 3.6.1 - /opt/homebrew/bin/fish
  Binaries:
    Node: 20.12.1 - /opt/homebrew/opt/node@20/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 10.5.0 - /opt/homebrew/opt/node@20/bin/npm
    pnpm: 9.4.0 - ~/Library/pnpm/pnpm
    bun: 1.1.3 - ~/.bun/bin/bun
  Browsers:
    Chrome: 126.0.6478.127
    Safari: 17.5
  npmPackages:
    @builder.io/qwik: file:../../test/qwik/packages/qwik/dist => 1.6.0-dev20240709174338 
    @builder.io/qwik-city: file:../../test/qwik/packages/qwik-city/lib => 1.6.0-dev20240709174338 
    typescript: 5.1.6 => 5.1.6 
    undici: 5.22.1 => 5.22.1 
    vite: 4.4.0 => 4.4.0

Additional Information

No response

genki commented 4 months ago

Sorry the reproduction url was wrong. Fixed.

genki commented 4 months ago

If you use the "id" attribute instead of the "class", the result is to be expected behaviour. Here's an example. https://stackblitz.com/edit/qwik-starter-vnvcex?file=src%2Froutes%2Fhome%2Findex.tsx Please check out the "id" is not "top" but now "home".

This means the priority of the attribute is not consistent and depending on its kind.

JerryWu1234 commented 4 months ago

let me try it

JerryWu1234 commented 4 months ago

image I think it's right if ID

JerryWu1234 commented 4 months ago

but class is indeed a problem. I will fix it

genki commented 4 months ago

@JerryWu1234 Thanks!

maiieul commented 4 months ago

@genki @JerryWu1234 this seems to have been fixed in 1.7.1. So I'm closing for now. Please feel free to ping me to re-open if I'm wrong.

JerryWu1234 commented 4 months ago

@genki @JerryWu1234 this seems to have been fixed in 1.7.1. So I'm closing for now. Please feel free to ping me to re-open if I'm wrong.

Could you give me how you do it? I knew that problem is Optimizer rust part of. I just curious how you fix it.

wmertens commented 4 months ago

I'm not sure if it's fixed in 1.7.1 but it should be fixed in 2.0