Skip to content

Commit

Permalink
fix: ensure unique classNames for different orders of pseudo class an…
Browse files Browse the repository at this point in the history
…d elements
  • Loading branch information
nmn committed Oct 15, 2024
1 parent a55cd69 commit 4d36e77
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 77 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
28 changes: 14 additions & 14 deletions packages/shared/__tests__/stylex-create-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,22 +640,22 @@ describe('stylex-create-test', () => {
"default": {
"$$css": true,
"::before_color": "xvg9oe5",
":hover_::before_color": "x1qdmp1q x18ezmze x14o3fp0",
":hover_::before_color": "xewn9if x1gobd9t x1lvqgcc",
},
},
{
"x14o3fp0": {
"ltr": ".x14o3fp0:hover::before:active{color:yellow}",
"priority": 8300,
"x1gobd9t": {
"ltr": ".x1gobd9t:hover::before:hover{color:green}",
"priority": 8260,
"rtl": null,
},
"x18ezmze": {
"ltr": ".x18ezmze:hover::before:hover{color:green}",
"priority": 8260,
"x1lvqgcc": {
"ltr": ".x1lvqgcc:hover::before:active{color:yellow}",
"priority": 8300,
"rtl": null,
},
"x1qdmp1q": {
"ltr": ".x1qdmp1q:hover::before{color:red}",
"xewn9if": {
"ltr": ".xewn9if:hover::before{color:red}",
"priority": 8130,
"rtl": null,
},
Expand All @@ -667,19 +667,19 @@ describe('stylex-create-test', () => {
},
{
"default": {
"x14o3fp0": [
"x1gobd9t": [
":hover",
"::before",
":active",
":hover",
"color",
],
"x18ezmze": [
"x1lvqgcc": [
":hover",
"::before",
":hover",
":active",
"color",
],
"x1qdmp1q": [
"xewn9if": [
":hover",
"::before",
"default",
Expand Down
18 changes: 9 additions & 9 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('');

// TODO: remove the `null` fallback.
// This will cause all classNames to change so will need to be careful.
const modifierHashString = atRuleHashString + pseudoHashString || 'null';

const stringToHash = Array.isArray(value)
? dashedKey + value.join(', ') + modifierHashString
: dashedKey + value + modifierHashString;
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 4d36e77

Please sign in to comment.