Skip to content

Commit

Permalink
fix: ensure unique classNames for pseudo classes & elems (#750)
Browse files Browse the repository at this point in the history
* fix: ensure unique classNames for different orders of pseudo class and elements
* chore: rebase onto PR with repro
* chore: better comments
* fix: variable names
  • Loading branch information
nmn authored Oct 16, 2024
1 parent ffca010 commit 1357106
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 85 deletions.
12 changes: 6 additions & 6 deletions packages/babel-plugin/__tests__/stylex-transform-create-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -781,11 +781,11 @@ describe('@stylexjs/babel-plugin', () => {
var _inject2 = _inject;
import stylex from 'stylex';
_inject2(".x16oeupf::before{color:red}", 8000);
_inject2(".xeb2lg0:hover::before{color:blue}", 8130);
_inject2(".xzzpreb:hover::before{color:blue}", 8130);
export const styles = {
foo: {
"::before_color": "x16oeupf",
":hover_::before_color": "xeb2lg0",
":hover_::before_color": "xzzpreb",
$$css: true
}
};"
Expand Down Expand Up @@ -818,13 +818,13 @@ describe('@stylexjs/babel-plugin', () => {
var _inject2 = _inject;
import stylex from 'stylex';
_inject2(".x16oeupf::before{color:red}", 8000);
_inject2(".xeb2lg0:hover::before{color:blue}", 8130);
_inject2(".x18ezmze:hover::before:hover{color:green}", 8260);
_inject2(".xnj3kot:hover::before:active{color:purple}", 8300);
_inject2(".xzzpreb:hover::before{color:blue}", 8130);
_inject2(".x1gobd9t:hover::before:hover{color:green}", 8260);
_inject2(".xs8jp5:hover::before:active{color:purple}", 8300);
export const styles = {
foo: {
"::before_color": "x16oeupf",
":hover_::before_color": "xeb2lg0 x18ezmze xnj3kot",
":hover_::before_color": "xzzpreb x1gobd9t xs8jp5",
$$css: true
}
};"
Expand Down
38 changes: 19 additions & 19 deletions packages/shared/__tests__/stylex-create-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,50 +640,50 @@ describe('stylex-create-test', () => {
"default": {
"$$css": true,
"::before_color": "x16oeupf",
":hover_::before_color": "xeb2lg0 x18ezmze x14o3fp0",
":hover_::before_color": "xzzpreb x1gobd9t x1lvqgcc",
},
},
{
"x14o3fp0": {
"ltr": ".x14o3fp0:hover::before:active{color:yellow}",
"priority": 8300,
"rtl": null,
},
"x16oeupf": {
"ltr": ".x16oeupf::before{color:red}",
"priority": 8000,
"rtl": null,
},
"x18ezmze": {
"ltr": ".x18ezmze:hover::before:hover{color:green}",
"x1gobd9t": {
"ltr": ".x1gobd9t:hover::before:hover{color:green}",
"priority": 8260,
"rtl": null,
},
"xeb2lg0": {
"ltr": ".xeb2lg0:hover::before{color:blue}",
"x1lvqgcc": {
"ltr": ".x1lvqgcc:hover::before:active{color:yellow}",
"priority": 8300,
"rtl": null,
},
"xzzpreb": {
"ltr": ".xzzpreb:hover::before{color:blue}",
"priority": 8130,
"rtl": null,
},
},
{
"default": {
"x14o3fp0": [
":hover",
"x16oeupf": [
"::before",
":active",
"color",
],
"x16oeupf": [
"x1gobd9t": [
":hover",
"::before",
":hover",
"color",
],
"x18ezmze": [
"x1lvqgcc": [
":hover",
"::before",
":hover",
":active",
"color",
],
"xeb2lg0": [
"xzzpreb": [
":hover",
"::before",
"default",
Expand All @@ -695,7 +695,7 @@ describe('stylex-create-test', () => {
`);
});

test.skip('transforms nested pseudo-classes within pseudo elements', () => {
test('transforms nested pseudo-classes within pseudo elements', () => {
const [beforeHover] = styleXCreate({
default: {
'::before': {
Expand All @@ -721,7 +721,7 @@ describe('stylex-create-test', () => {
const hoverBeforeClass = hoverBefore.default[':hover_::before_color'];

expect(beforeHoverClass).toMatchInlineSnapshot('"xeb2lg0"');
expect(hoverBeforeClass).toMatchInlineSnapshot('"xeb2lg0"');
expect(hoverBeforeClass).toMatchInlineSnapshot('"xzzpreb"');

expect(beforeHoverClass).not.toEqual(hoverBeforeClass);
});
Expand Down
24 changes: 12 additions & 12 deletions packages/shared/src/convert-to-className.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ import dashify from './utils/dashify';
import transformValue from './transform-value';
import { generateRule } from './generate-css-rule';
import { defaultOptions } from './utils/default-options';
import { arraySort } from './utils/object-utils';
import * as messages from './messages';
import { sortAtRules, sortPseudos } from './utils/rule-utils';

// This function takes a single style rule and transforms it into a CSS rule.
// [color: 'red'] => ['color', 'classname-for-color-red', CSSRULE{ltr, rtl, priority}]
Expand All @@ -33,7 +33,7 @@ export function convertStyleToClassName(
const [key, rawValue] = objEntry;
const dashedKey = dashify(key);

let value = Array.isArray(rawValue)
let value: string | $ReadOnlyArray<string> = Array.isArray(rawValue)
? rawValue.map((eachValue) => transformValue(key, eachValue, options))
: transformValue(key, rawValue, options);

Expand All @@ -44,17 +44,17 @@ export function convertStyleToClassName(
value = variableFallbacks(value);
}

const sortedPseudos = arraySort(pseudos ?? []);
const sortedAtRules = arraySort(atRules ?? []);
const sortedPseudos = sortPseudos(pseudos ?? []);
const sortedAtRules = sortAtRules(atRules ?? []);

const atRuleHashString = sortedPseudos.join('');
const pseudoHashString = sortedAtRules.join('');
const pseudoHashString = sortedPseudos.join('');
const atRuleHashString = sortedAtRules.join('');

const modifierHashString = atRuleHashString + pseudoHashString || 'null';

const stringToHash = Array.isArray(value)
? dashedKey + value.join(', ') + modifierHashString
: dashedKey + value + modifierHashString;
// NOTE: 'null' is used to keep existing hashes stable.
// This should be removed in a future version.
const modifierHashString = pseudoHashString + atRuleHashString || 'null';
const valueAsString = Array.isArray(value) ? value.join(', ') : value;
const stringToHash = dashedKey + valueAsString + modifierHashString;

// NOTE: '<>' is used to keep existing hashes stable.
// This should be removed in a future version.
Expand All @@ -69,7 +69,7 @@ export function convertStyleToClassName(

export default function variableFallbacks(
values: $ReadOnlyArray<string>,
): Array<string> {
): $ReadOnlyArray<string> {
const firstVar = values.findIndex(
(val) => val.startsWith('var(') && val.endsWith(')'),
);
Expand Down
51 changes: 3 additions & 48 deletions packages/shared/src/preprocess-rules/PreRule.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import type { InjectableStyle, StyleXOptions } from '../common-types';
import type { IncludedStyles } from '../stylex-include';

import { convertStyleToClassName } from '../convert-to-className';
import { arrayEquals, arraySort } from '../utils/object-utils';
import { arrayEquals } from '../utils/object-utils';
import { sortAtRules, sortPseudos } from '../utils/rule-utils';

export type ClassesToOriginalPaths = {
+[className: string]: $ReadOnlyArray<string>,
Expand Down Expand Up @@ -65,50 +66,6 @@ export class PreIncludedStylesRule implements IPreRule {
}
}

// a comparator function that sorts strings alphabetically
// but where `default` always comes first
const stringComparator = (a: string, b: string): number => {
if (a === 'default') {
return -1;
}
if (b === 'default') {
return 1;
}
return a.localeCompare(b);
};

const sortPseudos = (
pseudos: $ReadOnlyArray<string>,
): $ReadOnlyArray<string> => {
if (pseudos.length < 2) {
return pseudos;
}

return pseudos
.reduce(
(acc, pseudo) => {
if (pseudo.startsWith('::')) {
return [...acc, pseudo];
}

const lastElement = acc[acc.length - 1];
const allButLast = acc.slice(0, acc.length - 1);
if (Array.isArray(lastElement)) {
return [...allButLast, [...lastElement, pseudo]];
} else {
return [...allButLast, lastElement, [pseudo]].filter(Boolean);
}
},
[] as $ReadOnlyArray<string | $ReadOnlyArray<string>>,
)
.flatMap((pseudo) => {
if (Array.isArray(pseudo)) {
return arraySort(pseudo, stringComparator);
}
return [pseudo];
});
};

export class PreRule implements IPreRule {
+property: string;
+value: string | number | $ReadOnlyArray<string | number>;
Expand All @@ -126,14 +83,12 @@ export class PreRule implements IPreRule {

get pseudos(): $ReadOnlyArray<string> {
const unsortedPseudos = this.keyPath.filter((key) => key.startsWith(':'));

return sortPseudos(unsortedPseudos);
// return arraySort(unsortedPseudos, stringComparator);
}

get atRules(): $ReadOnlyArray<string> {
const unsortedAtRules = this.keyPath.filter((key) => key.startsWith('@'));
return arraySort(unsortedAtRules);
return sortAtRules(unsortedAtRules);
}

compiled(
Expand Down
58 changes: 58 additions & 0 deletions packages/shared/src/utils/rule-utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow strict
*/

import { arraySort } from './object-utils';

export const sortPseudos = (
pseudos: $ReadOnlyArray<string>,
): $ReadOnlyArray<string> => {
if (pseudos.length < 2) {
return pseudos;
}

return pseudos
.reduce(
(acc, pseudo) => {
if (pseudo.startsWith('::')) {
return [...acc, pseudo];
}

const lastElement = acc[acc.length - 1];
const allButLast = acc.slice(0, acc.length - 1);
if (Array.isArray(lastElement)) {
return [...allButLast, [...lastElement, pseudo]];
} else {
return [...allButLast, lastElement, [pseudo]].filter(Boolean);
}
},
[] as $ReadOnlyArray<string | $ReadOnlyArray<string>>,
)
.flatMap((pseudo) => {
if (Array.isArray(pseudo)) {
return arraySort(pseudo, stringComparator);
}
return [pseudo];
});
};

export const sortAtRules = (
atRules: $ReadOnlyArray<string>,
): $ReadOnlyArray<string> => arraySort(atRules);

// a comparator function that sorts strings alphabetically
// but where `default` always comes first
const stringComparator = (a: string, b: string): number => {
if (a === 'default') {
return -1;
}
if (b === 'default') {
return 1;
}
return a.localeCompare(b);
};

0 comments on commit 1357106

Please sign in to comment.