From cfac4483e1b35e398d0338ebaed92a1283dc3060 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Fri, 14 Aug 2026 15:47:45 +0300 Subject: [PATCH 1/2] fix(compiler): scope single-definition variable inlining to the declaring block A custom property is scoped to the elements its declaring rule matches, and a class selector cannot promise that a consuming rule matches the same element. `inlineVariables` counted declarations across the whole stylesheet and, at exactly one, folded the value into every `var()` reference in the sheet and deleted the declaring rule. .parent { --x: 10px; } .child { width: var(--x); } `.child` compiles to `width: 10` although it is not a descendant of `.parent`, and `.parent` emits no rule at all. Adding an unrelated second declaration of `--x` anywhere in the sheet changes `.child` back to a runtime read - so one rule's output depends on the existence of another it has no relationship with. Fold only into uses in the same block that declares the variable, and remove the declaration only when nothing outside that block reads it. The same-block case Tailwind v4 actually emits keeps folding. --- src/compiler/inline-variables.ts | 144 +++++++++++++++++++++++++++++-- 1 file changed, 139 insertions(+), 5 deletions(-) diff --git a/src/compiler/inline-variables.ts b/src/compiler/inline-variables.ts index 609760c1..3dd7e837 100644 --- a/src/compiler/inline-variables.ts +++ b/src/compiler/inline-variables.ts @@ -19,6 +19,14 @@ export function inlineVariables( } } + // A custom property is scoped to the elements its declaring rule matches, and + // a class selector cannot promise that a consuming rule matches the same + // element. So a single-definition variable may only be folded into uses in the + // SAME declaration block, and its declaration may only be removed when nothing + // outside that block reads it — otherwise a descendant that legitimately + // inherits the value at runtime finds it gone. + const scope = collectVariableScope(stylesheet, vars); + stylesheet.rules = stylesheet.rules.map(function checkRule(rule) { switch (rule.type) { case "custom": @@ -48,7 +56,7 @@ export function inlineVariables( case "keyframes": rule.value.keyframes = rule.value.keyframes.map((keyframe) => { keyframe.declarations = - replaceDeclarationBlock(keyframe.declarations, vars) ?? + replaceDeclarationBlock(keyframe.declarations, vars, scope) ?? keyframe.declarations; return keyframe; @@ -58,6 +66,7 @@ export function inlineVariables( rule.value.declarations = replaceDeclarationBlock( rule.value.declarations, vars, + scope, ); rule.value.rules = rule.value.rules?.flatMap((rule) => checkRule(rule)); @@ -65,7 +74,7 @@ export function inlineVariables( return rule; case "nested-declarations": rule.value.declarations = - replaceDeclarationBlock(rule.value.declarations, vars) ?? {}; + replaceDeclarationBlock(rule.value.declarations, vars, scope) ?? {}; return rule; case "supports": rule.value.rules = rule.value.rules.flatMap((rule) => checkRule(rule)); @@ -85,18 +94,27 @@ export function inlineVariables( function replaceDeclarationBlock( block: DeclarationBlock | undefined, vars: Map, + scope: VariableScope, ) { if (!block) return; + // Only the variables this block itself declares are foldable into this + // block's own uses. + const foldable = new Map(); + for (const name of scope.declaredBy.get(block) ?? []) { + const info = vars.get(name); + if (info) foldable.set(name, info); + } + block.declarations = block.declarations ?.map((decl) => { - return replaceDeclaration(decl, vars); + return replaceDeclaration(decl, foldable, scope); }) .filter((d) => !!d); block.importantDeclarations = block.importantDeclarations ?.map((decl) => { - return replaceDeclaration(decl, vars); + return replaceDeclaration(decl, foldable, scope); }) .filter((d) => !!d); @@ -106,6 +124,7 @@ function replaceDeclarationBlock( function replaceDeclaration( declaration: Declaration, vars: Map, + scope: VariableScope, ) { if ( declaration.property !== "unparsed" && @@ -114,7 +133,13 @@ function replaceDeclaration( return declaration; } - if (declaration.property === "custom" && vars.has(declaration.value.name)) { + // The declaration is only removable once every use of it has been folded, + // which is true exactly when nothing outside its own block reads it. + if ( + declaration.property === "custom" && + vars.has(declaration.value.name) && + !scope.readOutsideDeclaringBlock.has(declaration.value.name) + ) { return; } @@ -207,3 +232,112 @@ function flattenVar( vars.set(name, varInfo); } + +/** + * Which block declares each single-definition variable, and whether anything + * outside that block reads it. + * + * Both halves are needed because they gate different things: the first decides + * where a value may be folded, the second decides whether the declaration may be + * removed. A variable declared in one block and read in another is foldable + * nowhere and removable never — the runtime has to resolve it against the + * element's own inherited scope, which is the only place that answer exists. + */ +interface VariableScope { + readonly declaredBy: Map>; + readonly readOutsideDeclaringBlock: Set; +} + +/** Every `var(--name)` read in a token tree. */ +function collectReads(part: TokenOrValue, into: Set) { + if (part.type === "var") { + into.add(part.value.name.ident); + for (const fallback of part.value.fallback ?? []) { + collectReads(fallback, into); + } + } else if (part.type === "function") { + for (const argument of part.value.arguments) { + collectReads(argument, into); + } + } else if (part.type === "unresolved-color") { + // A colour function's channels can carry var() reads too. + for (const value of Object.values(part.value)) { + if (Array.isArray(value)) { + for (const entry of value) collectReads(entry as TokenOrValue, into); + } + } + } +} + +function collectVariableScope( + stylesheet: StyleSheet, + vars: Map, +): VariableScope { + const declaredBy = new Map>(); + const declaringBlock = new Map(); + const readsByBlock = new Map>(); + + const visitBlock = (block: DeclarationBlock | undefined) => { + if (!block) return; + const declared = new Set(); + const read = new Set(); + for (const declaration of [ + ...(block.declarations ?? []), + ...(block.importantDeclarations ?? []), + ]) { + if (declaration.property === "custom") { + if (vars.has(declaration.value.name)) { + declared.add(declaration.value.name); + declaringBlock.set(declaration.value.name, block); + } + for (const part of declaration.value.value) collectReads(part, read); + } else if (declaration.property === "unparsed") { + for (const part of declaration.value.value) collectReads(part, read); + } + } + declaredBy.set(block, declared); + readsByBlock.set(block, read); + }; + + const visitRule = (rule: StyleSheet["rules"][number]): void => { + switch (rule.type) { + case "style": + visitBlock(rule.value.declarations); + for (const nested of rule.value.rules ?? []) { + visitRule(nested); + } + return; + case "nested-declarations": + visitBlock(rule.value.declarations); + return; + case "keyframes": + for (const keyframe of rule.value.keyframes) { + visitBlock(keyframe.declarations); + } + return; + case "media": + case "supports": + case "layer-block": + case "container": + for (const nested of rule.value.rules) { + visitRule(nested); + } + return; + default: + return; + } + }; + + for (const rule of stylesheet.rules) visitRule(rule); + + const readOutsideDeclaringBlock = new Set(); + for (const [block, read] of readsByBlock) { + for (const name of read) { + if (vars.has(name) && declaringBlock.get(name) !== block) { + readOutsideDeclaringBlock.add(name); + } + } + } + + return { declaredBy, readOutsideDeclaringBlock }; +} From b77fafb2518f2f8facf1ed32899bfbac6c4635ff Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Sat, 15 Aug 2026 00:17:17 +0300 Subject: [PATCH 2/2] fix(compiler): fold a variable only where its value is provable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Folding a single-declaration custom property into a `var()` asserts that every element the consuming rule matches holds that value. One declaration does not show that. `canFold` names the two cases that do, and either is enough: - the declaration is in a universal, unconditional scope (`:root`, `:host`, `html`, `*`), so every element holds it whatever else it matches; or - the reference is in the block that declares it, so any element the rule matches holds it by matching that rule. A query that switches the declaration off switches the reference off with it, so the two cannot disagree. Neither holds across two rules, so `.a { --x: red }` with `.b { color: var(--x) }` is left to the runtime. The same terms apply inside a variable's own value: `.a { --x: var(--y) }` no longer takes `.b`'s `--y` and carries it to elements that never matched `.b`. Four further scoping holes close with it: - A conditional `:root` is not universal. Whether `@media`/`@supports`/ `@container` applies is decided elsewhere, so a declaration inside one cannot be folded into a rule outside it. - A property registered `inherits: false` is not inherited, so a universal declaration of it reaches only the element it is written on. - Pruning multi-declaration candidates now happens before anything is flattened. Doing it lazily made the fold depend on whether the reference was written above the declarations or below them, and above them the LOSING cascade value was folded in. - A block-scoped declaration survives its own fold. It was folded into its own block only, and a descendant still inherits it at runtime — including one styled by a stylesheet compiled separately, which this pass cannot see. Whether a fold happened must not be observable, so two places where the unfolded value differed from the folded one are normalised: - `parseUnparsed` tested `args` for truthiness, which dropped `--v: 0` and `--v: 0px` entirely: the variable reached the runtime undeclared and the declaration reading it rendered nothing at all. - A `` reaching the runtime through the token path serialised as `16 / 9` where the property parser produces `16/9`, and never collapsed a square ratio to `1`. Three expectations in `variables.test.tsx` change, all the same case: a class-scoped variable read from another rule now resolves at runtime, and a runtime value carries the token the author wrote because no property is in hand to canonicalise it. React Native reads the name as the colour. --- .../compiler/inline-variables.test.ts | 238 +++++++++++++ .../native/variable-fold-parity.test.tsx | 264 +++++++++++++++ .../native/variable-inlining.test.tsx | 158 +++++++++ src/__tests__/native/variables.test.tsx | 13 +- src/compiler/declarations.ts | 50 ++- src/compiler/inline-variables.ts | 316 ++++++++++++------ 6 files changed, 923 insertions(+), 116 deletions(-) create mode 100644 src/__tests__/compiler/inline-variables.test.ts create mode 100644 src/__tests__/native/variable-fold-parity.test.tsx create mode 100644 src/__tests__/native/variable-inlining.test.tsx diff --git a/src/__tests__/compiler/inline-variables.test.ts b/src/__tests__/compiler/inline-variables.test.ts new file mode 100644 index 00000000..116c321a --- /dev/null +++ b/src/__tests__/compiler/inline-variables.test.ts @@ -0,0 +1,238 @@ +import type { + ReactNativeCssStyleSheet, + StyleRule, +} from "react-native-css/compiler"; +import { compile } from "react-native-css/compiler"; + +/** + * Folding a single-definition custom property into its use sites asserts that + * every element the consuming rule matches holds that value. These tests pin + * where that assertion is provable and where it is not. + * + * Compiler plane. `../native/variable-inlining.test.tsx` renders the same + * stylesheets, because a fold is a compiler decision whose only observable is + * what the runtime paints. + */ + +function rulesFor( + sheet: ReactNativeCssStyleSheet, + className: string, +): StyleRule[] { + return sheet.s?.find(([name]) => name === className)?.[1] ?? []; +} + +/** The value folded into `property`, or undefined if nothing was folded. */ +function foldedValue( + sheet: ReactNativeCssStyleSheet, + className: string, + property: string, +) { + for (const rule of rulesFor(sheet, className)) { + for (const declaration of rule.d ?? []) { + if (!Array.isArray(declaration) && property in declaration) { + return declaration[property]; + } + } + } + return undefined; +} + +/** Whether the class resolves at least one `var()` at runtime. */ +function readsAtRuntime(sheet: ReactNativeCssStyleSheet, className: string) { + return rulesFor(sheet, className).some((rule) => rule.dv === 1); +} + +/** The value the class declares for `--`, or undefined if it declares none. */ +function declaredValue( + sheet: ReactNativeCssStyleSheet, + className: string, + name: string, +) { + for (const rule of rulesFor(sheet, className)) { + for (const [declared, value] of rule.v ?? []) { + if (declared === name) return value; + } + } + return undefined; +} + +describe("a variable is folded only where its value is provable", () => { + test("a class-scoped variable does not reach another rule", () => { + const sheet = compile( + `.parent { --x: 10px; } .child { width: var(--x); }`, + ).stylesheet(); + + // An element carrying only `.child` never matched `.parent`, so it has no + // `--x` and `width: 10` would be an invention. + expect(foldedValue(sheet, "child", "width")).toBeUndefined(); + expect(readsAtRuntime(sheet, "child")).toBe(true); + // ...and the declaration has to survive for the runtime to find it. + expect(declaredValue(sheet, "parent", "x")).toBe(10); + }); + + test("the same block is provable, and still folds", () => { + const sheet = compile(`.a { --x: 10px; width: var(--x); }`).stylesheet(); + + expect(foldedValue(sheet, "a", "width")).toBe(10); + expect(readsAtRuntime(sheet, "a")).toBe(false); + }); + + test("a block-scoped declaration survives its own fold", () => { + // The fold reached this block's references and no others. A descendant + // inherits `--x` at runtime — including one styled by a stylesheet compiled + // separately, which this pass cannot see and must not assume away. + const sheet = compile(`.a { --x: 10px; width: var(--x); }`).stylesheet(); + + expect(declaredValue(sheet, "a", "x")).toBe(10); + }); + + test.each([":root", ":host", "*", "html"])( + "%s is universal, so its variables fold anywhere", + (selector) => { + const sheet = compile( + `${selector} { --x: 10px; } .child { width: var(--x); }`, + ).stylesheet(); + + expect(foldedValue(sheet, "child", "width")).toBe(10); + expect(readsAtRuntime(sheet, "child")).toBe(false); + }, + ); + + test.each([ + [":root .theme", "a descendant of the root is not the root"], + ["div", "every element descends from html, from no other element name"], + [":hover", "a state is not a scope"], + [".theme", "a class is the case this whole rule exists for"], + ])("%s is not universal — %s", (selector) => { + const sheet = compile( + `${selector} { --x: 10px; } .child { width: var(--x); }`, + ).stylesheet(); + + expect(foldedValue(sheet, "child", "width")).toBeUndefined(); + expect(readsAtRuntime(sheet, "child")).toBe(true); + }); + + test.each([ + [":root", 10], + [".theme", undefined], + ])("a nested `&` under %s takes its parent's answer", (parent, expected) => { + // `&` adds no constraint of its own, so the scope is whatever the rule it + // is nested in already was. + const sheet = compile( + `${parent} { & { --x: 10px; } } .child { width: var(--x); }`, + ).stylesheet(); + + expect(foldedValue(sheet, "child", "width")).toBe(expected); + }); + + test("@layer does not narrow a universal scope", () => { + // The shape Tailwind emits for its theme. + const sheet = compile( + `@layer theme { :root, :host { --x: 10px; } } .child { width: var(--x); }`, + ).stylesheet(); + + expect(foldedValue(sheet, "child", "width")).toBe(10); + }); +}); + +describe("a conditional scope is not a universal one", () => { + test.each([ + ["@media (min-width: 1px)", "media"], + ["@supports (display: flex)", "supports"], + ["@container (min-width: 1px)", "container"], + ])("%s wrapping :root", (atRule) => { + const sheet = compile( + `${atRule} { :root { --x: 10px; } } .child { width: var(--x); }`, + ).stylesheet(); + + // Whether the declaration applies is decided somewhere this pass cannot + // see, so `.child` cannot be told it holds the value. + expect(foldedValue(sheet, "child", "width")).toBeUndefined(); + expect(readsAtRuntime(sheet, "child")).toBe(true); + }); + + test("a conditional block still folds into itself", () => { + // A query that switches the declaration off switches the reference off with + // it, so the two cannot disagree. + const sheet = compile( + `@media (min-width: 1px) { .a { --x: 10px; width: var(--x); } }`, + ).stylesheet(); + + expect(foldedValue(sheet, "a", "width")).toBe(10); + }); +}); + +describe("a registered property is a second definition", () => { + test("inherits: false makes a universal declaration non-universal", () => { + // `--x` does not inherit, so an element that is not the root holds the + // REGISTERED initial value, not the one `:root` declares. + const sheet = compile( + `@property --x { syntax: ""; inherits: false; initial-value: 10px; } + :root { --x: 20px; } + .b { width: var(--x); }`, + ).stylesheet(); + + expect(foldedValue(sheet, "b", "width")).toBeUndefined(); + expect(readsAtRuntime(sheet, "b")).toBe(true); + }); + + test("inherits: true leaves the universal fold alone", () => { + const sheet = compile( + `@property --x { syntax: ""; inherits: true; initial-value: 10px; } + :root { --x: 20px; } + .b { width: var(--x); }`, + ).stylesheet(); + + expect(foldedValue(sheet, "b", "width")).toBe(20); + }); +}); + +describe("a variable's own value is held to the same terms", () => { + test("a nested reference does not cross a block boundary", () => { + const sheet = compile( + `.a { --x: var(--y); width: var(--x); } .b { --y: 10px; }`, + ).stylesheet(); + + // `--y` belongs to `.b`. Folding it into `.a`'s value carries it to every + // element matching `.a`, none of which need ever have matched `.b` — and + // `.a`'s own `width` is where that lands, because `--x` IS foldable there. + expect(foldedValue(sheet, "a", "width")).toBeUndefined(); + expect(readsAtRuntime(sheet, "a")).toBe(true); + expect(declaredValue(sheet, "a", "x")).toStrictEqual([{}, "var", "y", 1]); + }); + + test("a nested reference in the same block folds", () => { + const sheet = compile( + `.a { --y: 10px; --x: var(--y); width: var(--x); }`, + ).stylesheet(); + + expect(declaredValue(sheet, "a", "x")).toBe(10); + expect(foldedValue(sheet, "a", "width")).toBe(10); + }); + + test("a universal nested reference folds anywhere", () => { + const sheet = compile( + `:root { --y: 10px; } .a { --x: var(--y); }`, + ).stylesheet(); + + expect(declaredValue(sheet, "a", "x")).toBe(10); + }); +}); + +test("the fold does not depend on the order the rules are written in", () => { + // `--y` has two declarations, so it is not a candidate at all, and `2px` is + // the one that wins. Pruning a candidate only when its own turn came round + // made that depend on whether `--x` was written above the declarations or + // below them — and above them, the LOSING `1px` was folded in. + const declarations = `:root { --y: 1px; } :root { --y: 2px; }`; + const reference = `.a { --x: var(--y); width: var(--x); }`; + + const varFirst = compile(`${reference} ${declarations}`).stylesheet(); + const varLast = compile(`${declarations} ${reference}`).stylesheet(); + + for (const sheet of [varFirst, varLast]) { + expect(foldedValue(sheet, "a", "width")).toBeUndefined(); + expect(readsAtRuntime(sheet, "a")).toBe(true); + expect(declaredValue(sheet, "a", "x")).toStrictEqual([{}, "var", "y", 1]); + } +}); diff --git a/src/__tests__/native/variable-fold-parity.test.tsx b/src/__tests__/native/variable-fold-parity.test.tsx new file mode 100644 index 00000000..caaa2fe5 --- /dev/null +++ b/src/__tests__/native/variable-fold-parity.test.tsx @@ -0,0 +1,264 @@ +import { render } from "@testing-library/react-native"; +import type { TokenOrValue } from "lightningcss"; +import { View } from "react-native-css/components/View"; +import { registerCSS, testID } from "react-native-css/jest"; +import { StyleCollection } from "react-native-css/native"; + +/** + * Whether the compiler folds a single-definition custom property into its use + * sites is an OPTIMISATION. It must not be observable: the same stylesheet + * compiled with the fold on and with the fold off has to render the same style. + * + * The census below is exhaustive over the value kinds lightningcss can put + * inside a custom property, by construction — both records are keyed by the + * lightningcss union itself, so a new member fails to compile until it is + * censused here. An empty list is a kind that cannot be the whole value of a + * custom property a React Native style consumes, and says why. + */ +type ValueKind = TokenOrValue["type"]; +type RawTokenKind = Extract["value"]["type"]; + +interface Shape { + /** The property that reads the variable. */ + readonly property: string; + /** The custom property's value. */ + readonly value: string; + /** + * A divergence this change does not close, with the reason it is out of + * reach. Pinned as an exact pair so it cannot drift silently in either + * direction: a NEW divergence fails, and so does one that quietly goes away. + */ + readonly knownDivergence?: { + readonly folded: Record; + readonly unfolded: Record; + readonly because: string; + }; +} + +/** + * A named CSS colour reaches the second pass as a bare ident, because + * lightningcss only promotes a custom property's value to a colour node when it + * is not expressible as one. The folded path then parses it under the consuming + * property and canonicalises it; the unfolded path stores it under no property + * at all, so it cannot. Normalising the store instead would break the reverse + * case — `font-family: var(--v)` over `--v: red` renders "red" on BOTH paths + * today and would start rendering "#f00" on one of them. + */ +const namedColour = (folded: string, unfolded: string) => ({ + folded: { color: folded }, + unfolded: { color: unfolded }, + because: + "a named colour is an ident until a property gives it a type; the variable store has no property", +}); + +const TOKEN_CENSUS: Record = { + "ident": [ + { property: "position", value: "absolute" }, + { + property: "color", + value: "red", + knownDivergence: namedColour("#f00", "red"), + }, + { + property: "color", + value: "transparent", + knownDivergence: namedColour("#0000", "transparent"), + }, + { + property: "color", + value: "rebeccapurple", + knownDivergence: namedColour("#639", "rebeccapurple"), + }, + ], + "string": [{ property: "font-family", value: '"Inter"' }], + "number": [ + { property: "flex-grow", value: "2" }, + { property: "width", value: "0" }, + { property: "opacity", value: "0" }, + { property: "z-index", value: "3" }, + ], + "percentage": [{ property: "width", value: "50%" }], + "dimension": [ + { + property: "transition-duration", + value: "3s", + knownDivergence: { + folded: {}, + unfolded: { transitionDuration: 3000 }, + because: + "folding routes the duration to the animation system, which paints nothing; not folding leaks it into the static style. A transition gap, not a variable one", + }, + }, + ], + "delim": [ + { property: "aspect-ratio", value: "16 / 9" }, + // A square ratio has a second canonical form, and the property parser + // picks it. + { property: "aspect-ratio", value: "3 / 3" }, + ], + "hash": [{ property: "color", value: "#123456" }], + // Not reachable as the whole value of a custom property a style consumes. + "at-keyword": [], + "id-hash": [], + "unquoted-url": [], + "white-space": [], + "comment": [], + "colon": [], + "semicolon": [], + "comma": [], + "include-match": [], + "dash-match": [], + "prefix-match": [], + "suffix-match": [], + "substring-match": [], + "cdo": [], + "cdc": [], + "function": [], + "parenthesis-block": [], + "square-bracket-block": [], + "curly-bracket-block": [], + "bad-url": [], + "bad-string": [], + "close-parenthesis": [], + "close-square-bracket": [], + "close-curly-bracket": [], +}; + +const VALUE_CENSUS: Record = { + "token": Object.values(TOKEN_CENSUS).flat(), + "color": [ + { property: "color", value: "#12345678" }, + { property: "color", value: "rgba(255, 0, 0, 0.5)" }, + { property: "color", value: "oklch(63.7% 0.237 25.331)" }, + ], + "length": [ + { property: "width", value: "10px" }, + { property: "width", value: "0px" }, + { property: "margin-top", value: "-4px" }, + { property: "width", value: "1rem" }, + ], + "angle": [ + { + property: "rotate", + value: "45deg", + knownDivergence: { + folded: { transform: [{ rotateZ: "45deg" }] }, + unfolded: { transform: [{ rotate: "45deg" }] }, + because: + "`rotate` is renamed to `rotateZ` by the static parser and left alone by the runtime one; fold-independent, and closing it is a runtime shorthand change", + }, + }, + ], + "function": [ + { property: "width", value: "calc(10px + 2px)" }, + { property: "transform", value: "translateX(10px)" }, + ], + "var": [ + { + property: "color", + value: "var(--inner)", + knownDivergence: namedColour("#008080", "teal"), + }, + ], + // Both paths agree, and both are wrong about the red channel by a factor of + // 255. That is `parseUnresolvedColor`'s bug, not a fold one, and parity is + // all this census claims. + "unresolved-color": [ + { property: "color", value: "rgb(255 0 0 / var(--alpha))" }, + ], + // Not reachable, or not a style value React Native consumes. + "url": [], + "env": [], + "time": [], + "resolution": [], + "dashed-ident": [], + "animation-name": [], +}; + +/** + * Shorthands are a second fold-independent class: the static parser expands + * them into their longhands, the runtime one hands React Native the raw list. + * Same shape as the `rotate` entry above, kept apart because it is a family. + */ +const SHORTHANDS: readonly Shape[] = ( + [ + [ + "margin", + "1px 2px", + "marginTop", + "marginBottom", + "marginLeft", + "marginRight", + ], + [ + "padding", + "1px 2px", + "paddingTop", + "paddingBottom", + "paddingLeft", + "paddingRight", + ], + [ + "border-width", + "1px 2px", + "borderTopWidth", + "borderBottomWidth", + "borderLeftWidth", + "borderRightWidth", + ], + ] as const +).map(([property, value, top, bottom, left, right]) => ({ + property, + value, + knownDivergence: { + folded: { [top]: 1, [bottom]: 1, [left]: 2, [right]: 2 }, + unfolded: { [property.replace("-w", "W")]: [1, 2] }, + because: + "the static parser expands the shorthand, the runtime one does not; fold-independent, and closing it needs a runtime shorthand handler", + }, +})); + +const SHAPES: readonly Shape[] = [ + ...Object.values(VALUE_CENSUS).flat(), + ...SHORTHANDS, +]; + +function renderShape( + shape: Shape, + inlineVariables: boolean, +): Record { + // `--inner` and `--alpha` back the two shapes whose value is itself a + // reference; every other shape ignores them. + const css = `.a { --inner: teal; --alpha: 0.5; --v: ${shape.value}; ${shape.property}: var(--v); }`; + StyleCollection.styles.clear(); + registerCSS(css, inlineVariables ? {} : { inlineVariables: false }); + const { style } = render().getByTestId( + testID, + ).props as { style?: Record }; + + return style ?? {}; +} + +test("the census covers something", () => { + // Deriving the table from a union buys a drift failure at the cost of a + // vacuity one: every list could be empty and every case below would vanish. + expect(SHAPES.length).toBeGreaterThan(20); +}); + +describe.each(SHAPES)("$property: var(--v) over $value", (shape) => { + test("the fold decision is unobservable", () => { + const folded = renderShape(shape, true); + const unfolded = renderShape(shape, false); + + if (shape.knownDivergence) { + expect(folded).toStrictEqual(shape.knownDivergence.folded); + expect(unfolded).toStrictEqual(shape.knownDivergence.unfolded); + return; + } + + // A shape that renders nothing on both paths agrees vacuously, which is + // exactly how a typo in the census would pass. + expect(folded).not.toStrictEqual({}); + expect(unfolded).toStrictEqual(folded); + }); +}); diff --git a/src/__tests__/native/variable-inlining.test.tsx b/src/__tests__/native/variable-inlining.test.tsx new file mode 100644 index 00000000..ac2fb49c --- /dev/null +++ b/src/__tests__/native/variable-inlining.test.tsx @@ -0,0 +1,158 @@ +import { act, render, screen } from "@testing-library/react-native"; +import { vars } from "react-native-css"; +import { View } from "react-native-css/components/View"; +import { registerCSS, testID } from "react-native-css/jest"; + +import { dimensions } from "../../native/reactivity"; + +/** + * Every test names its own custom properties. `:root` variables live in a + * store the jest `beforeEach` does not reset, so a shared name would let one + * test decide another's result depending on the order they ran in. + * + * Native plane. `../compiler/inline-variables.test.ts` asserts the same folds + * against the IR; these assert what the runtime paints, because an IR that is + * scoped correctly and a screen that renders correctly are two claims. + */ + +test("a class-scoped variable does not leak into an unrelated rule", () => { + registerCSS(`.parent { --leak-a: 10px; } .child { width: var(--leak-a); }`); + + render(); + + // No `.parent` anywhere above it, so there is no `--leak-a` to read. + expect(screen.getByTestId(testID).props.style).toStrictEqual({}); +}); + +test("a class-scoped variable still reaches a descendant", () => { + registerCSS(`.parent { --leak-b: 10px; } .child { width: var(--leak-b); }`); + + render( + + + , + ); + + // The declaration has to survive the fold for this to resolve at all. + expect(screen.getByTestId(testID).props.style).toStrictEqual({ width: 10 }); +}); + +test("a variable's own value does not leak across blocks", () => { + registerCSS( + `.a { --nest-a: var(--nest-b); width: var(--nest-a); } .b { --nest-b: 10px; }`, + ); + + render(); + + // `--nest-b` belongs to `.b`. An element carrying only `.a` never had it. + expect(screen.getByTestId(testID).props.style).toStrictEqual({}); +}); + +test("a conditional :root does not fold into an unconditional rule", () => { + registerCSS(` + @media (min-width: 1000px) { :root { --cond-off: 10px; } } + .child { width: var(--cond-off); } + `); + + act(() => { + dimensions.set({ width: 500, height: 500, scale: 1, fontScale: 1 }); + }); + render(); + + // The declaration does not apply at this width, so neither does the value. + expect(screen.getByTestId(testID).props.style).toStrictEqual({}); +}); + +test("a conditional :root still applies when its query matches", () => { + registerCSS(` + @media (min-width: 1000px) { :root { --cond-on: 10px; } } + .child { width: var(--cond-on); } + `); + + act(() => { + dimensions.set({ width: 1200, height: 500, scale: 1, fontScale: 1 }); + }); + render(); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ width: 10 }); +}); + +test("a property registered with inherits: false resolves at runtime", () => { + registerCSS(` + @property --registered { syntax: ""; inherits: false; initial-value: 10px; } + :root { --registered: 20px; } + .b { width: var(--registered); } + `); + + render(); + + // KNOWN LIMIT, pinned rather than claimed correct. CSS says `--registered` + // does not inherit, so `.b` holds the registered initial value 10, not `:root`'s + // 20. The compiler now refuses to fold it — the sibling compiler test pins + // that — but the runtime carries no notion of a non-inherited custom + // property, so it still resolves 20 from the root scope. Modelling + // `inherits: false` in the variable store is what closes this, and refusing + // the fold is the prerequisite rather than the fix. + expect(screen.getByTestId(testID).props.style).toStrictEqual({ width: 20 }); +}); + +test("a runtime write reaches a rule that reads the variable", () => { + registerCSS(`.parent { --write-a: red; } .child { color: var(--write-a); }`); + + render( + + + , + ); + + // An inline write outranks a class declaration, and folding the class value + // into `.child` would have made that unrepresentable. + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + color: "blue", + }); +}); + +test("a runtime write cannot reach a reference in the declaring block", () => { + registerCSS(`.a { --write-b: red; color: var(--write-b); }`); + + render( + , + ); + + // KNOWN LIMIT, pinned rather than claimed correct. CSS says the inline write + // wins and this should be blue. The value was folded at build time, so no + // read remains for the write to affect. Closing it means not folding a + // same-block reference either, which is the whole optimisation; `blue` + // is recoverable today with `inlineVariables: { exclude: [...] }`. + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + color: "#f00", + }); +}); + +test("excluding a variable restores the runtime write", () => { + registerCSS(`.a { --write-c: red; color: var(--write-c); }`, { + inlineVariables: { exclude: ["--write-c"] }, + }); + + render( + , + ); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + color: "blue", + }); +}); diff --git a/src/__tests__/native/variables.test.tsx b/src/__tests__/native/variables.test.tsx index e61340b5..161602b3 100644 --- a/src/__tests__/native/variables.test.tsx +++ b/src/__tests__/native/variables.test.tsx @@ -171,8 +171,12 @@ test("can apply and set new variables", () => { expect(screen.getByTestId(testIDs.two).props.style).toStrictEqual({ color: "#f00", }); + // `--another-var` belongs to `.my-class`, so `.another-class` reaches it by + // inheritance at runtime rather than by a fold. A runtime value carries the + // token the author wrote: no property is in hand to canonicalise it, and + // React Native reads the name as the colour. expect(screen.getByTestId(testIDs.three).props.style).toStrictEqual({ - color: "#008000", + color: "green", }); }); @@ -198,8 +202,9 @@ test("variables will be inherited", () => { , ); + // Inherited from `.green` at runtime, which is the whole subject of the test. expect(screen.getByTestId(testIDs.three).props.style).toStrictEqual({ - color: "#008000", + color: "green", }); }); @@ -269,5 +274,7 @@ test("variable overriding with classes", () => { ); const component = screen.getByTestId(testID); - expect(component.props.style).toStrictEqual({ color: "#f00" }); + // `--tier-500` belongs to `.tier-red`; `.test` reads it by inheritance. The + // `:root` tier folds into `.tier-red`'s value, so what inherits is `red`. + expect(component.props.style).toStrictEqual({ color: "red" }); }); diff --git a/src/compiler/declarations.ts b/src/compiler/declarations.ts index 13013642..86474972 100644 --- a/src/compiler/declarations.ts +++ b/src/compiler/declarations.ts @@ -1024,6 +1024,38 @@ export function parseCustomDeclaration( } } +/** + * A `` written as three tokens, e.g. `16 / 9`. + * + * Only a two-term ratio is one: anything longer that happens to contain a `/` + * is not a ratio and has no canonical form to normalise it to. + */ +function isRatioGroup( + group: readonly unknown[], +): group is [number, "/", number] { + return ( + group.length === 3 && + typeof group[0] === "number" && + group[1] === "/" && + typeof group[2] === "number" + ); +} + +/** + * The same value `parseAspectRatio` produces for the same ratio. + * + * Whether a `` reaches the runtime through the property parser or + * through this one is decided by whether the compiler folded the variable + * holding it, and that decision must not be visible in the value. + */ +function ratioDescriptor([width, , height]: [ + number, + "/", + number, +]): StyleDescriptor { + return width === height ? 1 : `${width}/${height}`; +} + export function reduceParseUnparsed( tokenOrValues: TokenOrValue[], builder: StylesheetBuilder, @@ -1069,17 +1101,8 @@ export function reduceParseUnparsed( } else { return [first]; } - } else if ( - // This is a special case for values - group.includes("/") && - group.every((item) => - typeof item === "string" && item === "/" - ? item - : typeof item === "number", - ) - ) { - // eslint-disable-next-line @typescript-eslint/no-base-to-string - return [group.join(" ")]; + } else if (isRatioGroup(group)) { + return [ratioDescriptor(group)]; } else { return [group]; } @@ -1144,7 +1167,10 @@ export function parseUnparsed( property, allowAuto, ); - if (!args) return; + // `0` and `false` are values, not absences. Testing truthiness here dropped + // `--v: 0` on the floor, so a variable holding it reached the runtime + // undeclared and the declaration reading it rendered nothing at all. + if (args === undefined) return; if (Array.isArray(args) && args.length === 1) { return args[0]; } else if ( diff --git a/src/compiler/inline-variables.ts b/src/compiler/inline-variables.ts index 3dd7e837..31575c7e 100644 --- a/src/compiler/inline-variables.ts +++ b/src/compiler/inline-variables.ts @@ -1,32 +1,50 @@ import type { Declaration, DeclarationBlock, + Rule, + Selector, StyleSheet, TokenOrValue, } from "lightningcss"; import type { UniqueVarInfo } from "./compiler.types"; +/** + * Folds a custom property that the stylesheet declares exactly once into the + * `var()` references that read it, so the runtime never has to resolve it. + * + * Folding a value into a rule ASSERTS that every element the rule matches holds + * that value. A single declaration is not enough to know that — a custom + * property is scoped to the elements its declaring rule matches, and a class + * selector cannot promise that a consuming rule matches the same element: + * + * .parent { --x: 10px } .child { width: var(--x) } + * + * An element carrying only `.child` has no `--x` at all, so `width: 10px` is + * wrong for it. `canFold` names the two cases where the assertion IS provable. + */ export function inlineVariables( stylesheet: StyleSheet, vars: Map, ) { + // A second declaration is a cascade this pass cannot resolve — which of the + // two wins depends on the element — so only a property declared exactly once + // is a candidate at all. Pruning happens BEFORE anything is flattened, + // because a value being flattened reads this same map: leaving a + // multi-declaration variable in it until its own turn came round made the + // fold depend on the order the properties happened to be written in. for (const [name, info] of [...vars]) { if (info.count !== 1) { vars.delete(name); - } else { - flattenVar(name, vars); } } - // A custom property is scoped to the elements its declaring rule matches, and - // a class selector cannot promise that a consuming rule matches the same - // element. So a single-definition variable may only be folded into uses in the - // SAME declaration block, and its declaration may only be removed when nothing - // outside that block reads it — otherwise a descendant that legitimately - // inherits the value at runtime finds it gone. const scope = collectVariableScope(stylesheet, vars); + for (const name of [...vars.keys()]) { + flattenVar(name, vars, scope); + } + stylesheet.rules = stylesheet.rules.map(function checkRule(rule) { switch (rule.type) { case "custom": @@ -91,6 +109,45 @@ export function inlineVariables( return stylesheet; } +/** + * Where each single-definition custom property is declared, and whether that + * place is one every element inherits from. + * + * The two halves answer different questions. `declaringBlocks` decides whether + * a reference sits in the same block as the declaration; `universalNames` + * decides whether the declaration reaches every element regardless. + */ +interface VariableScope { + readonly declaringBlocks: Map; + readonly universalNames: Set; +} + +/** + * Whether the value of `name` may be folded into a `var()` written in `block`. + * + * Two cases are provable, and either is enough: + * + * - the declaration is in a universal, unconditional scope, so every element + * holds the property whatever else it matches; or + * - the reference is in the block that declares it, so any element the rule + * matches holds the property by matching that rule. Whether the rule applies + * at all does not matter: a query that switches the declaration off switches + * the reference off with it. + * + * Neither holds across two rules, which is why `.a { --x: red }` with + * `.b { color: var(--x) }` is left to the runtime. + */ +function canFold( + name: string, + block: DeclarationBlock | undefined, + scope: VariableScope, +) { + return ( + scope.universalNames.has(name) || + (block !== undefined && scope.declaringBlocks.get(name) === block) + ); +} + function replaceDeclarationBlock( block: DeclarationBlock | undefined, vars: Map, @@ -98,23 +155,15 @@ function replaceDeclarationBlock( ) { if (!block) return; - // Only the variables this block itself declares are foldable into this - // block's own uses. - const foldable = new Map(); - for (const name of scope.declaredBy.get(block) ?? []) { - const info = vars.get(name); - if (info) foldable.set(name, info); - } - block.declarations = block.declarations ?.map((decl) => { - return replaceDeclaration(decl, foldable, scope); + return replaceDeclaration(decl, vars, block, scope); }) .filter((d) => !!d); block.importantDeclarations = block.importantDeclarations ?.map((decl) => { - return replaceDeclaration(decl, foldable, scope); + return replaceDeclaration(decl, vars, block, scope); }) .filter((d) => !!d); @@ -124,6 +173,7 @@ function replaceDeclarationBlock( function replaceDeclaration( declaration: Declaration, vars: Map, + block: DeclarationBlock, scope: VariableScope, ) { if ( @@ -133,18 +183,21 @@ function replaceDeclaration( return declaration; } - // The declaration is only removable once every use of it has been folded, - // which is true exactly when nothing outside its own block reads it. + // A universal declaration has been folded into every reference there is, so + // nothing is left to read it. A block-scoped one is KEPT: it was folded only + // into its own block, and a descendant still inherits it at runtime — + // including a descendant styled by a stylesheet compiled separately, which + // this pass cannot see and must not assume away. if ( declaration.property === "custom" && vars.has(declaration.value.name) && - !scope.readOutsideDeclaringBlock.has(declaration.value.name) + scope.universalNames.has(declaration.value.name) ) { return; } declaration.value.value = declaration.value.value.flatMap((part) => { - return flattenPart(part, vars); + return flattenPart(part, vars, block, scope); }); return declaration; @@ -153,19 +206,22 @@ function replaceDeclaration( function flattenPart( part: TokenOrValue, vars: Map, + block: DeclarationBlock | undefined, + scope: VariableScope, ): TokenOrValue | TokenOrValue[] { if (part.type === "var") { - const varInfo = vars.get(part.value.name.ident); + const name = part.value.name.ident; + const varInfo = vars.get(name); - if (!varInfo) { + if (!varInfo || !canFold(name, block, scope)) { part.value.fallback = part.value.fallback?.flatMap((arg) => { - return flattenPart(arg, vars); + return flattenPart(arg, vars, block, scope); }); return part; } else if (varInfo.value === undefined) { const fallback = part.value.fallback?.flatMap((arg) => { - return flattenPart(arg, vars); + return flattenPart(arg, vars, block, scope); }); return fallback ?? []; @@ -174,7 +230,7 @@ function flattenPart( return varInfo.value; } else if (part.type === "function") { part.value.arguments = part.value.arguments.flatMap((arg) => { - return flattenPart(arg, vars); + return flattenPart(arg, vars, block, scope); }); } @@ -184,6 +240,7 @@ function flattenPart( function flattenVar( name: string, vars: Map, + scope: VariableScope, seen = new Set(), ) { if (seen.has(name)) { @@ -198,18 +255,27 @@ function flattenVar( return; } + // A value is flattened FOR the block that declares it, because that is the + // only block it is ever substituted into. A `var()` inside it is therefore + // held to the same terms as every other reference written in that block — + // otherwise `.a { --x: var(--y) }` quietly takes `.b`'s `--y` and carries it + // to elements that never matched `.b`. + const declaringBlock = scope.declaringBlocks.get(name); + let varInfoValue = varInfo.value?.flatMap((part) => { if (part.type === "var") { - const name = part.value.name.ident; + const nestedName = part.value.name.ident; - flattenVar(name, vars, seen); + flattenVar(nestedName, vars, scope, seen); - const nestedVarInfo = vars.get(part.value.name.ident); - if (nestedVarInfo?.value) { - return nestedVarInfo.value; + if (canFold(nestedName, declaringBlock, scope)) { + const nestedVarInfo = vars.get(nestedName); + if (nestedVarInfo?.value) { + return nestedVarInfo.value; + } } } - return flattenPart(part, vars); + return flattenPart(part, vars, declaringBlock, scope); }); // If the variable is shorthand for "initial", substitute it for undefined @@ -233,94 +299,107 @@ function flattenVar( vars.set(name, varInfo); } -/** - * Which block declares each single-definition variable, and whether anything - * outside that block reads it. - * - * Both halves are needed because they gate different things: the first decides - * where a value may be folded, the second decides whether the declaration may be - * removed. A variable declared in one block and read in another is foldable - * nowhere and removable never — the runtime has to resolve it against the - * element's own inherited scope, which is the only place that answer exists. - */ -interface VariableScope { - readonly declaredBy: Map>; - readonly readOutsideDeclaringBlock: Set; +/** Where a declaration block sits, as the annotating walk descends. */ +interface RuleScope { + /** Every element matches the enclosing selector. */ + readonly universal: boolean; + /** Enclosed by a query whose result this pass does not know. */ + readonly conditional: boolean; } -/** Every `var(--name)` read in a token tree. */ -function collectReads(part: TokenOrValue, into: Set) { - if (part.type === "var") { - into.add(part.value.name.ident); - for (const fallback of part.value.fallback ?? []) { - collectReads(fallback, into); - } - } else if (part.type === "function") { - for (const argument of part.value.arguments) { - collectReads(argument, into); - } - } else if (part.type === "unresolved-color") { - // A colour function's channels can carry var() reads too. - for (const value of Object.values(part.value)) { - if (Array.isArray(value)) { - for (const entry of value) collectReads(entry as TokenOrValue, into); - } - } - } -} +const ROOT_SCOPE: RuleScope = { universal: false, conditional: false }; function collectVariableScope( stylesheet: StyleSheet, vars: Map, ): VariableScope { - const declaredBy = new Map>(); - const declaringBlock = new Map(); - const readsByBlock = new Map>(); + const scope: VariableScope = { + declaringBlocks: new Map(), + universalNames: new Set(), + }; + + // A property registered with `inherits: false` is NOT inherited, so a + // universal declaration of it reaches only the element it is written on — + // every descendant sees the registered initial value instead. Recording it + // before the walk keeps `:root { --x: 20px }` from being read as universal. + const notInherited = new Set(); + // Top level only, which is the whole of the compiler's `@property` support: + // an `@property` nested in an at-rule registers no initial value either, so + // there is nothing there for this to disagree with. + for (const rule of stylesheet.rules) { + if (rule.type === "property" && !rule.value.inherits) { + notInherited.add(rule.value.name); + } + } - const visitBlock = (block: DeclarationBlock | undefined) => { + const annotateBlock = ( + block: DeclarationBlock | undefined, + ruleScope: RuleScope, + ) => { if (!block) return; - const declared = new Set(); - const read = new Set(); + + const universal = ruleScope.universal && !ruleScope.conditional; + for (const declaration of [ ...(block.declarations ?? []), ...(block.importantDeclarations ?? []), ]) { - if (declaration.property === "custom") { - if (vars.has(declaration.value.name)) { - declared.add(declaration.value.name); - declaringBlock.set(declaration.value.name, block); - } - for (const part of declaration.value.value) collectReads(part, read); - } else if (declaration.property === "unparsed") { - for (const part of declaration.value.value) collectReads(part, read); + if (declaration.property !== "custom") continue; + + const { name } = declaration.value; + if (!vars.has(name)) continue; + + scope.declaringBlocks.set(name, block); + if (universal && !notInherited.has(name)) { + scope.universalNames.add(name); } } - declaredBy.set(block, declared); - readsByBlock.set(block, read); }; - const visitRule = (rule: StyleSheet["rules"][number]): void => { + const annotateRule = (rule: Rule, ruleScope: RuleScope): void => { switch (rule.type) { - case "style": - visitBlock(rule.value.declarations); - for (const nested of rule.value.rules ?? []) { - visitRule(nested); + case "style": { + const nested: RuleScope = { + ...ruleScope, + universal: isUniversalScope(rule.value.selectors, ruleScope), + }; + annotateBlock(rule.value.declarations, nested); + for (const child of rule.value.rules ?? []) { + annotateRule(child, nested); } return; + } case "nested-declarations": - visitBlock(rule.value.declarations); + // The enclosing style rule's own declarations, so they carry its scope. + annotateBlock(rule.value.declarations, ruleScope); return; case "keyframes": + // `@keyframes` is only ever reached at the top level or inside an + // at-rule, never inside a style rule, so the scope it carries is + // already non-universal. A keyframe declares animation values rather + // than a scope another rule can rely on, but its own block still folds + // into itself. for (const keyframe of rule.value.keyframes) { - visitBlock(keyframe.declarations); + annotateBlock(keyframe.declarations, ruleScope); } return; case "media": case "supports": - case "layer-block": case "container": - for (const nested of rule.value.rules) { - visitRule(nested); + // Whether these rules apply at all is decided elsewhere — at runtime + // for a media or container query, at build time for `@supports` — so a + // declaration inside one is never unconditional, however universal its + // selector. + for (const child of rule.value.rules) { + annotateRule(child, { + universal: false, + conditional: true, + }); + } + return; + case "layer-block": + for (const child of rule.value.rules) { + annotateRule(child, ruleScope); } return; default: @@ -328,16 +407,51 @@ function collectVariableScope( } }; - for (const rule of stylesheet.rules) visitRule(rule); + for (const rule of stylesheet.rules) annotateRule(rule, ROOT_SCOPE); - const readOutsideDeclaringBlock = new Set(); - for (const [block, read] of readsByBlock) { - for (const name of read) { - if (vars.has(name) && declaringBlock.get(name) !== block) { - readOutsideDeclaringBlock.add(name); - } + return scope; +} + +/** + * Whether a selector list names a scope every element inherits from. + * + * ONE such selector is enough, so the list is tested with `some`: `:root, :host` + * — the shape Tailwind emits for its theme — is universal because `:root` is, + * whatever `:host` matches. + * + * A nested rule is universal only if both it and the rule it is nested in are, + * because its selectors are relative to that parent. A nested selector that is + * nothing but `&` adds no constraint of its own and takes the parent's answer. + */ +function isUniversalScope(selectors: Selector[], ruleScope: RuleScope) { + return selectors.some((selector) => { + const meaningful = selector.filter( + (component) => component.type !== "nesting", + ); + + if (meaningful.length === 0) { + return ruleScope.universal; + } + + if (meaningful.length !== 1) { + // Anything compound or combined is narrower than the whole document: + // `:root .theme` matches only descendants of an element with that class. + return false; } - } - return { declaredBy, readOutsideDeclaringBlock }; + const [component] = meaningful; + + switch (component?.type) { + case "universal": + return true; + case "type": + // Every element descends from `html`. No other element name can be + // relied on, and none of them compile to a rule here anyway. + return component.name === "html"; + case "pseudo-class": + return component.kind === "root" || component.kind === "host"; + default: + return false; + } + }); }