stylex icon indicating copy to clipboard operation
stylex copied to clipboard

[babel-plugin] Can generate redundant class names for pseudo-classes

Open necolas opened this issue 8 months ago • 2 comments

Describe the issue

Nested pseudo-classes have a pre-determined order, which means 2 different ways of authoring a style produce the same CSS. But the compiler unnecessarily inserts a duplicate class name into the compiled style object.

  const { code, metadata } = transform(`
    import * as stylex from '@stylexjs/stylex';
    export const styles = stylex.create({
      root: {
        color: {
          ':hover': {
            ':active':'red',
          },
          ':active': {
            ':hover':'red',
          },
        },
      },
    });
  `);

  expect(code).toMatchInlineSnapshot(`
    "import * as stylex from '@stylexjs/stylex';
    export const styles = {
      root: {
        kMwMTN: "xa2ikkt xa2ikkt",
        $$css: true
      }
    };"
  `);

Expected behavior

No duplicate class name

Steps to reproduce

See example above

Test case

See example above

Additional comments

No response

necolas avatar Apr 16 '25 19:04 necolas

@necolas , some changes to the compiler,

How about this?

// babel-plugin/src/shared/stylex-create.js
for (const [key, value] of compiledNamespaceTuples) {
      // Remove nulls
      const classNameTuples: $ReadOnlyArray<TClassNameTuples> = value
        .map((v) => (Array.isArray(v) ? v : null))
        .filter(Boolean);

      classNameTuples.forEach(([_className, _, classesToOriginalPath]) => {
        Object.assign(classPathsInNamespace, classesToOriginalPath);
      });

      // const className =
      //   classNameTuples.map(([className]) => className).join(' ') || null;
      // namespaceObj[key] = className;
      const classNames = classNameTuples.map(([className]) => className);
      const uniqueClassNames = new Set(classNames);
      const className = Array.from(uniqueClassNames).join(' ') || null;
      namespaceObj[key] = className;
      console.log(className)
      for (const [className, injectable] of classNameTuples) {
        if (injectedStyles[className] == null) {
          injectedStyles[className] = injectable;
        }
      }
    }

This can be easily changed by removing duplicate classnames with Set() and inserting them back into the array!

and I think the associated test code will likely need to be changed as well.

jeongminsang avatar May 06 '25 04:05 jeongminsang

@jeongminsang thanks! I put this fix up in #1047 and listed you as co-author

necolas avatar May 06 '25 22:05 necolas