jantimon / next-yak

🦀 Zero-runtime CSS-in-JS powered by Rust. Write styled-components syntax, get build-time CSS extraction and full RSC compatibility
https://yak.js.org
139 stars 4 forks source link

Import of selector fails when it hasn't any base styles #213

Open Mad-Kat opened 1 week ago

Mad-Kat commented 1 week ago

When importing components as selectors, we always attempt to generate an import for the component's base class, which causes issues with components that only have dynamic classes based on props.

Currently

When using an imported component as a selector:

import { Label } from "./other";

const OverwriteLabel = styled.div`
  ${Label}{
    color: pink;
  }
`

we generate:

const OverwriteLabel = /*YAK Extracted CSS:
.OverwriteLabel {
  --yak-css-import: url("./other:Label",selector) {
    color: pink;
  }
}
*/ /*#__PURE__*/ styled.div(__styleYak.OverwriteLabel);

which results in this CSS:

.OverwriteLabel {
  :global(.other_Label__l1FMk) {
    color: pink;
  }
}

Problem

The generated CSS assumes there's a base class named Label in the imported component's CSS. However, this assumption fails when the imported component only has dynamic classes based on props:

export const Label = styled.label<{ $variant: "primary" | "secondary" }>`
  ${({ $variant }) => $variant === "primary" ? css`
    color: red;
  ` : css`
    color: blue;
  `}
`;

which results in this CSS:

.Label__ {
  color: red;
}
.Label__1 {
  color: blue;
}

In this case, there is no base .Label class to target, causing the selector to fail.

Proposed Solutions

  1. Generate Base Class Always Always generate a base class for components, regardless of whether they have static styles Pros: Maintains backwards compatibility Cons: Additional CSS overhead

  2. Throw Descriptive Error Detect when a component lacks a base class and throw an error with an explanation of the issue and suggestions for solutions like "add base styles to the component" or "split into separate components" Pros: Clearer developer experience Cons: Breaking change for the migration from styled-components to next-yak

jantimon commented 5 days ago

thanks for the report - I think we should simply always generate a base class

the overhead for the html and css is rather tiny because there are probably just a limited cases where we wouldn't use a base class

because of this issue I came up with an idea for a potential future default case optimisation - take this example:

const Button = styled.button<{$active: boolean}>`
  ${({$active}) => $active 
    ? css`color: red;` 
    : css`color: blue;`
  }
`;

Instead of three classes:

.Button-abc123 {}
.Button-abc123--active { color: red; }
.Button-abc123--inactive { color: blue; }

We could optimize to:

.Button-abc123 { color: blue; }
.Button-abc123--active { color: red; }

but let's solve the immediate issue first with a simple fix