kripod/otion

Re-inserted conditional rules shouldn’t cause style duplication

kripod opened this issue · 2 comments

Motivation

When re-inserting a style for precedence management, the old rule isn’t removed from the stylesheet. This may cause some duplication when media or feature queries are used frequently.

eddyw commented

👋 When a conditional rule is re-inserted, it shouldn't be removed or duplicated but it should be generated with a different classname. This also applies when they aren't re-inserted in this case:

Take for example this:

export default function IndexPage(): JSX.Element {
  return (
    <React.StrictMode>
      <h1>Hello, world!</h1>
      <p
        className={css({
          "@media": {
            "(min-width: 5px)": { color: "blue" },
            "(min-width: 4px)": { color: "green" },
          },
        })}
      >
        Conditional rule precedence management test #1, should be green
      </p>
    </React.StrictMode>
  );
}

It is green and generates the following CSS code:

@media(min-width:5px){._bxnxkd{color:blue}}
@media(min-width:4px){._107ye90{color:green}}

which is the correct order in which they were defined.
Now try this:

export default function IndexPage(): JSX.Element {
  return (
    <React.StrictMode>
      <h1>Hello, world!</h1>
      <p
        className={css({
          "@media": {
            "(min-width: 4px)": { color: "green" },
            "(min-width: 5px)": { color: "blue" },
          },
        })}
      >
        Conditional rule precedence management test #2, should be blue
      </p>
    </React.StrictMode>
  );
}

It is blue and generates again in the CSS in correct order:

@media(min-width:4px){._107ye90{color:green}}
@media(min-width:5px){._bxnxkd{color:blue}}

However, because re-inserted conditional rules have the same class-name, this happens:

export default function IndexPage(): JSX.Element {
  return (
    <React.StrictMode>
      <h1>Hello, world!</h1>
      <p
        className={css({
          "@media": {
            "(min-width: 5px)": { color: "blue" },
            "(min-width: 4px)": { color: "green" },
          },
        })}
      >
        Conditional rule precedence management test #1, should be green
      </p>
      <p
        className={css({
          "@media": {
            "(min-width: 4px)": { color: "green" },
            "(min-width: 5px)": { color: "blue" },
          },
        })}
      >
        Conditional rule precedence management test #2, should be blue
      </p>
    </React.StrictMode>
  );
}

Both are green and CSS is:

@media(min-width:5px){._bxnxkd{color:blue}}
@media(min-width:4px){._107ye90{color:green}}

The expected result should be:

@media(min-width:5px){._bxnxkd{color:blue}} /* SourceIndex: 0 */
@media(min-width:4px){._107ye90{color:green}} /* SourceIndex: 1 */
@media(min-width:4px){._NEW_CLS_0{color:green}} /* SourceIndex: 0 */
@media(min-width:5px){._NEW_CLS_1{color:blue}} /* SourceIndex: 1 */

I couldn't understand very well what createInstance does (the terms used could use some comments 😅 ) but ..
The hash method could also use precedence and ruleIndex to generate the classname (sort of like hash(declarations + precedence + ruleIndex || 0)), so you could always check if a rule has been inserted instead of re-inserting the rule and removing the old one which would break styling (as shown above)

Awesome, I really love this idea! It would allow conditional rules to be defined in any order without issues. (A mobile-first approach is still recommended and should possibly generate a shorter output, though.)

Your examples are well-detailed and will help me a lot when working on the implementation, possibly next week. Thank you so much! 🙌

As for the documentation, an overhaul would be nice to have, especially for developers like you, who dive deep into the details. The createInstance method works similarly to the concept of a class, as it returns the css and keyframes methods to be used throughout an application. A default instance is created and set up for a better developer experience. I'm not sure about the value of custom instances, though, as their only purpose is allowing injection of styles into <iframe> elements.