From 756e3209c68fb2cb478f6e8bbad1d9461c034c9a Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Sat, 15 Aug 2026 00:22:13 +0300 Subject: [PATCH 1/6] fix(native): cut circular variable resolution instead of blowing the stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A variable is handed to a descendant as an UNRESOLVED descriptor, so a value that names its own variable resolves back into itself. Each of these takes the render down with `RangeError: Maximum call stack size exceeded`: .parent { --a: red } .mid { --a: var(--a) } .child { color: var(--a) } .parent { --a: red } .mid { --a: var(--nope, var(--a)) } .child { color: var(--a) } .parent { --a: red } .mid { --a: var(--b); --b: var(--a) } .child { color: var(--a) } `varResolver` carried a `variableHistory` guard, but it could never fire. The set was destructured out of `options` with a `new Set()` default and never written back, so every invocation built its own empty one and the recursion never shared a history. The registration also sat AFTER the `if (name in variables)` early return — which is the branch a descendant takes, and therefore the branch the recursion runs through. The set now lives on `options`, so every nested resolve sees it, and a name is registered before any of its values are resolved. It is released in a `finally` once they are, which makes it a resolution STACK rather than a visited set: a genuine cycle is cut on re-entry, while a name read twice in one declaration (`box-shadow: var(--c) 1px 1px, var(--c) 2px 2px`) still resolves both times. --- src/__tests__/native/variables.test.tsx | 93 +++++++++++++++++++++++++ src/native/styles/variables.ts | 84 +++++++++++++--------- 2 files changed, 143 insertions(+), 34 deletions(-) diff --git a/src/__tests__/native/variables.test.tsx b/src/__tests__/native/variables.test.tsx index e61340b5..2021fec2 100644 --- a/src/__tests__/native/variables.test.tsx +++ b/src/__tests__/native/variables.test.tsx @@ -271,3 +271,96 @@ test("variable overriding with classes", () => { const component = screen.getByTestId(testID); expect(component.props.style).toStrictEqual({ color: "#f00" }); }); + +/** + * A variable is handed to a descendant as an UNRESOLVED descriptor, so a value + * that names its own variable resolves back into itself. Without a cycle guard + * that survives the recursion, the descendant blows the stack instead of + * rendering. + */ +describe("circular variables", () => { + const circularStylesheets: [name: string, css: string][] = [ + [ + "a variable whose value is itself", + `.parent { --a: red } .mid { --a: var(--a) } .child { color: var(--a) }`, + ], + [ + "a variable reached again through a fallback", + `.parent { --a: red } .mid { --a: var(--nope, var(--a)) } .child { color: var(--a) }`, + ], + [ + "two variables that name each other", + `.parent { --a: red } .mid { --a: var(--b); --b: var(--a) } .child { color: var(--a) }`, + ], + ]; + + test("the census is not empty", () => { + expect(circularStylesheets.length).toBeGreaterThan(0); + }); + + test.each(circularStylesheets)("%s renders", (_name, css) => { + registerCSS(css); + + render( + + + + + , + ); + + // The cycle has no value, so the declaration reading it resolves to nothing. + expect(screen.getByTestId(testID).props.style).toStrictEqual({}); + }); + + test("a variable read twice in ONE declaration is not mistaken for a cycle", () => { + // Both reads share one resolution pass, so the guard has to track names + // whose resolution is IN PROGRESS rather than names already seen. + // `inlineVariables` is off so the reads survive to runtime instead of being + // folded at compile time, as a provider or :root variable does. + registerCSS( + ` + .parent { --shadow-color: red } + .child { + box-shadow: + var(--shadow-color) 1px 1px, + var(--shadow-color) 2px 2px; + } + `, + { inlineVariables: false }, + ); + + render( + + + , + ); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + boxShadow: [ + { color: "red", offsetX: 1, offsetY: 1 }, + { color: "red", offsetX: 2, offsetY: 2 }, + ], + }); + }); + + test("a long non-circular chain still resolves", () => { + registerCSS( + ` + .parent { --a: var(--b); --b: var(--c); --c: var(--d); --d: red } + .child { color: var(--a) } + `, + { inlineVariables: false }, + ); + + render( + + + , + ); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + color: "red", + }); + }); +}); diff --git a/src/native/styles/variables.ts b/src/native/styles/variables.ts index af3d6ee2..e379131f 100644 --- a/src/native/styles/variables.ts +++ b/src/native/styles/variables.ts @@ -18,7 +18,6 @@ export function varResolver( renderGuards, inheritedVariables: variables = { [VAR_SYMBOL]: true }, inlineVariables, - variableHistory = new Set(), } = options; const args = fn[2]; @@ -41,48 +40,65 @@ export function varResolver( return; } - // If this recurses back to the same variable, we need to stop - if (variableHistory.has(name)) { + /** + * The names whose resolution is currently in progress, shared through + * `options` so every nested resolve below sees the same set. + * + * A variable's value can name the variable again — directly + * (`--a: var(--a)`), through a fallback, or around a longer chain — and a + * variable is handed to a descendant UNRESOLVED, so resolving it re-enters + * here with the same name and no base case. A name is registered before any + * of its values are resolved and removed once they are, which makes this a + * resolution STACK rather than a visited set: a genuine cycle is cut on + * re-entry, while a name read twice in sequence resolves both times. + */ + const namesBeingResolved = (options.variableHistory ??= new Set()); + + if (namesBeingResolved.has(name)) { return; } - if (name in variables) { - renderGuards?.push(["v", name, variables[name]]); - return resolve(variables[name]); - } + namesBeingResolved.add(name); - variableHistory.add(name); + try { + if (name in variables) { + renderGuards?.push(["v", name, variables[name]]); + return resolve(variables[name]); + } - let value = resolve(inlineVariables?.[name] as StyleDescriptor); - if (value !== undefined) { - options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; - options.inlineVariables[name] = value; + let value = resolve(inlineVariables?.[name] as StyleDescriptor); + if (value !== undefined) { + options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; + options.inlineVariables[name] = value; - return value; - } + return value; + } - value = resolve(variables[name]); - if (value !== undefined) { - renderGuards?.push(["v", name, value]); - options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; - options.inlineVariables[name] = value; + value = resolve(variables[name]); + if (value !== undefined) { + renderGuards?.push(["v", name, value]); + options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; + options.inlineVariables[name] = value; - return value; - } + return value; + } - value = resolve(get(universalVariables(name))); - if (value !== undefined) { - options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; - options.inlineVariables[name] = value; - return value; - } + value = resolve(get(universalVariables(name))); + if (value !== undefined) { + options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; + options.inlineVariables[name] = value; + return value; + } - value = resolve(get(rootVariables(name))); - if (value !== undefined) { - options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; - options.inlineVariables[name] = value; - return value; - } + value = resolve(get(rootVariables(name))); + if (value !== undefined) { + options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; + options.inlineVariables[name] = value; + return value; + } - return resolve(fallback); + return resolve(fallback); + } finally { + namesBeingResolved.delete(name); + } } From 9f039c888e0b9a7495d7b46a15bc57234f9a45d4 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Sun, 16 Aug 2026 12:30:49 +0300 Subject: [PATCH 2/6] test(native): make the circular-variable census falsifiable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every row in the census asserted `{}`, which is what the cycle guard produces AND what a dead variable resolver produces. Making `varResolver` return `undefined` unconditionally — every `var()` in the library dead — reddens 143 tests across the suite and left all three rows green. Each row now reads a non-cyclic `--unrelated` beside the cycle, so a row can only pass while resolution still works. The rows also did not compile to the shapes they described. A variable declared exactly once is substituted into its readers, so `.mid { --a: var(--b); --b: var(--a) }` folded to `.mid { --a: var(--a) }` and compiled to the same stylesheet as the first row — the census advertised three shapes and delivered two. Every name in a cycle is now declared twice, which is what makes the two-node row a two-node cycle. Two mutations of the guard survived the census and no longer do: - Returning the re-entering reference's fallback from the cut, against the spec sentence the guard quotes. The fallback sat on the OUTER `var()`, so the cut had none to return and the mutation was a no-op; it now sits on the reference that re-enters. - Emptying the whole stack in the `finally` rather than popping one frame. A name re-entered from two branches of ONE value separates those, and no test had that shape: `--a: var(--b) var(--c)` where both name `--a` recurses forever under `clear()`. Removal is now pinned from both sides — removing too little reddens the two reads in one `box-shadow`, too much reddens the diamond. Both public entry points that recurse without the guard get a test — `useUnstableNativeVariable` and `VariableContextProvider`, whose value type admits a `var()` reference. So does the compiler's own cycle guard, which nothing covered: disabling `flattenVar`'s `seen` set leaves the suite at the exact baseline while `.solo { --z: var(--z) }` recurses at compile time. `ResolveValueOptions.variableHistory` becomes `namesBeingResolved`, matching the local it feeds and what it holds — the names whose resolution is in progress, not the names already seen. The type is internal to `native/styles/` and is re-exported from no entry point. The comment on the `finally` had `options` threaded through the whole style calculation, which would refuse a variable read by a second declaration. `applyDeclarations` builds a fresh options object per declaration, so two declarations never share a stack; removing the `finally` reddens exactly one test in the suite, the two reads in one `box-shadow`. The comments now also record what the cut produces — the property loses its value, or keeps a truncated one where the cycle is part of a larger value — that an inherited name resolving to nothing swallows a reader's fallback, and that the guard bounds cycles only: a long enough non-circular chain still exhausts the stack, at a depth that varies with how deep it already is. --- src/__tests__/native/variables.test.tsx | 124 ++++++++++++++++++++-- src/compiler/inline-variables.ts | 10 ++ src/native/styles/resolve.ts | 3 +- src/native/styles/shorthands/animation.ts | 6 ++ src/native/styles/variables.ts | 41 +++++-- 5 files changed, 169 insertions(+), 15 deletions(-) diff --git a/src/__tests__/native/variables.test.tsx b/src/__tests__/native/variables.test.tsx index 2021fec2..a7a2ccb1 100644 --- a/src/__tests__/native/variables.test.tsx +++ b/src/__tests__/native/variables.test.tsx @@ -2,9 +2,15 @@ import { memo, useEffect } from "react"; import type { ViewProps } from "react-native"; import { render, screen } from "@testing-library/react-native"; -import { styled, VariableContextProvider } from "react-native-css"; +import { styled } from "react-native-css"; import { View } from "react-native-css/components/View"; import { registerCSS, testID } from "react-native-css/jest"; +// The native entry, so the provider's value type is the native +// `StyleDescriptor` one rather than web's `string | number`. +import { + useUnstableNativeVariable, + VariableContextProvider, +} from "react-native-css/native"; test("inline variable", () => { registerCSS(`.my-class { width: var(--my-var); --my-var: 10px; }`); @@ -279,18 +285,59 @@ test("variable overriding with classes", () => { * rendering. */ describe("circular variables", () => { - const circularStylesheets: [name: string, css: string][] = [ + /** + * Every row closes a cycle on `--a` and reads it from `.child`, beside a + * NON-cyclic `--unrelated` that has to keep resolving. The second read is + * what makes the assertion falsifiable: a row that only asserts the cycle + * produced nothing passes just as green when variable resolution is dead + * altogether. + * + * Every name is declared TWICE because the compiler substitutes a variable + * declared exactly once directly into its readers. A single declaration + * folds the cycle away before the runtime resolver these rows exist to + * exercise ever sees it, leaving a row that reads as one shape and compiles + * to another. + */ + const circularStylesheets: [ + name: string, + css: string, + style: Record, + ][] = [ [ "a variable whose value is itself", - `.parent { --a: red } .mid { --a: var(--a) } .child { color: var(--a) }`, + `.parent { --a: red; --unrelated: 1 } + .mid { --a: var(--a); --unrelated: 0.5 } + .child { color: var(--a); opacity: var(--unrelated) }`, + { opacity: 0.5 }, ], [ + // The inner `blue` is the point: a cycle is invalid at computed-value + // time, so the cut yields nothing rather than the fallback of the + // reference that re-entered it. "a variable reached again through a fallback", - `.parent { --a: red } .mid { --a: var(--nope, var(--a)) } .child { color: var(--a) }`, + `.parent { --a: red; --unrelated: 1 } + .mid { --a: var(--nope, var(--a, blue)); --unrelated: 0.5 } + .child { color: var(--a); opacity: var(--unrelated) }`, + { opacity: 0.5 }, ], [ "two variables that name each other", - `.parent { --a: red } .mid { --a: var(--b); --b: var(--a) } .child { color: var(--a) }`, + `.parent { --a: red; --b: blue; --unrelated: 1 } + .mid { --a: var(--b); --b: var(--a); --unrelated: 0.5 } + .child { color: var(--a); opacity: var(--unrelated) }`, + { opacity: 0.5 }, + ], + [ + // Two branches of ONE value re-enter the same name. Cutting the first + // branch has to pop only its own frame — a guard that empties the whole + // stack lets the second branch start over and recurse forever. + "one variable re-entered from two branches of one value", + `.parent { --a: red; --b: blue; --c: green; --unrelated: 1 } + .mid { --a: var(--b) var(--c); --b: var(--a); --c: var(--a); --unrelated: 0.5 } + .child { color: var(--a); opacity: var(--unrelated) }`, + // The cycle is only part of `--a`, so `color` keeps the surviving + // siblings rather than losing the declaration. + { color: [], opacity: 0.5 }, ], ]; @@ -298,7 +345,7 @@ describe("circular variables", () => { expect(circularStylesheets.length).toBeGreaterThan(0); }); - test.each(circularStylesheets)("%s renders", (_name, css) => { + test.each(circularStylesheets)("%s renders", (_name, css, style) => { registerCSS(css); render( @@ -309,8 +356,7 @@ describe("circular variables", () => { , ); - // The cycle has no value, so the declaration reading it resolves to nothing. - expect(screen.getByTestId(testID).props.style).toStrictEqual({}); + expect(screen.getByTestId(testID).props.style).toStrictEqual(style); }); test("a variable read twice in ONE declaration is not mistaken for a cycle", () => { @@ -363,4 +409,66 @@ describe("circular variables", () => { color: "red", }); }); + + test("a cycle in a variable declared ONCE is cut at compile time", () => { + // A variable declared once is substituted into its readers, so this cycle + // is closed by the compiler's own guard in `inline-variables.ts` and never + // reaches the resolution stack above. + registerCSS( + `.parent { --unrelated: 1 } + .mid { --unrelated: 0.5 } + .child { --z: var(--z); width: var(--z); opacity: var(--unrelated) }`, + ); + + render( + + + + + , + ); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + opacity: 0.5, + }); + }); + + test("useUnstableNativeVariable reads a cycle without recursing", () => { + registerCSS(`.parent { --a: red } .mid { --a: var(--a) }`); + + let read: unknown = "not read"; + + function Probe() { + read = useUnstableNativeVariable("--a"); + return ; + } + + render( + + + + + , + ); + + expect(read).toBeUndefined(); + }); + + test("VariableContextProvider accepts a self-referential value", () => { + // The provider's value type admits a `var()` reference, so a caller can + // hand it a variable that names itself. + registerCSS(`.child { color: var(--a); opacity: var(--unrelated) }`); + + render( + + + , + ); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ + opacity: 0.5, + }); + }); }); diff --git a/src/compiler/inline-variables.ts b/src/compiler/inline-variables.ts index 609760c1..cf14e851 100644 --- a/src/compiler/inline-variables.ts +++ b/src/compiler/inline-variables.ts @@ -161,6 +161,16 @@ function flattenVar( vars: Map, seen = new Set(), ) { + /** + * The compile-time counterpart of the resolution stack in + * `native/styles/variables.ts`. Substituting a variable into its readers + * follows the same references, so a self-referential value recurses here + * with no base case — this drops the name instead, which leaves the + * reference unsubstituted and hands the cycle to the runtime guard. + * + * A variable declared more than once is never inlined, so it reaches the + * runtime guard without passing through here at all. + */ if (seen.has(name)) { vars.delete(name); } diff --git a/src/native/styles/resolve.ts b/src/native/styles/resolve.ts index 8465e9b1..fdb6a10b 100644 --- a/src/native/styles/resolve.ts +++ b/src/native/styles/resolve.ts @@ -50,7 +50,8 @@ export type ResolveValueOptions = { inheritedVariables?: VariableContextValue; inlineVariables?: InlineVariable | undefined; renderGuards?: RenderGuard[]; - variableHistory?: Set; + /** The variable names whose resolution is currently in progress. */ + namesBeingResolved?: Set; /** Pass down to perform recursive calculations and avoid circular dependencies */ calculateProps?: typeof calculateProps; }; diff --git a/src/native/styles/shorthands/animation.ts b/src/native/styles/shorthands/animation.ts index 9b5cc39a..cbc86ef2 100644 --- a/src/native/styles/shorthands/animation.ts +++ b/src/native/styles/shorthands/animation.ts @@ -92,6 +92,12 @@ export const animation: StyleFunctionResolver = ( for (const [progress, declarations] of keyframes) { animation[progress] ??= {}; + /** + * Keyframe declarations resolve through a fresh options object, so the + * resolution stack in `../variables.ts` does not cross this boundary. The + * animation name is resolved and its frame popped before this runs, which + * leaves no live frame for a keyframe to re-enter. + */ const props = options.calculateProps?.( get, // Cast this into a StyleRule[] diff --git a/src/native/styles/variables.ts b/src/native/styles/variables.ts index e379131f..fdfa019a 100644 --- a/src/native/styles/variables.ts +++ b/src/native/styles/variables.ts @@ -42,18 +42,41 @@ export function varResolver( /** * The names whose resolution is currently in progress, shared through - * `options` so every nested resolve below sees the same set. + * `options` so every nested resolve below sees the same stack. * * A variable's value can name the variable again — directly * (`--a: var(--a)`), through a fallback, or around a longer chain — and a * variable is handed to a descendant UNRESOLVED, so resolving it re-enters - * here with the same name and no base case. A name is registered before any - * of its values are resolved and removed once they are, which makes this a - * resolution STACK rather than a visited set: a genuine cycle is cut on - * re-entry, while a name read twice in sequence resolves both times. + * here with the same name and no base case. + * + * A name is registered before any of its values are resolved and removed + * once they are, which makes this a resolution STACK rather than a visited + * set. Both halves are load-bearing: + * + * - Registering cuts a genuine cycle on re-entry. + * - Removing per frame keeps a name readable again once its own resolution + * has finished, which a name read twice within ONE declaration needs + * (`box-shadow: var(--c) 1px 1px, var(--c) 2px 2px`). Only the + * within-one-declaration case depends on it — `applyDeclarations` builds + * a fresh options object per declaration, so two declarations never share + * a stack. Emptying the whole stack instead would let a second branch of + * one value start the cycle over: `--a: var(--b) var(--c)` where both + * name `--a` recurses forever. + * + * This bounds CYCLES only. A non-circular chain long enough to exhaust the + * JS stack still throws, at a depth that varies with how deep the stack + * already is when resolution starts. */ - const namesBeingResolved = (options.variableHistory ??= new Set()); + const namesBeingResolved = (options.namesBeingResolved ??= new Set()); + /** + * A variable in a cycle is invalid at computed-value time, so the cut yields + * nothing rather than the fallback of the reference that re-entered it. + * + * The property reading it loses its value, or keeps a TRUNCATED one where + * the cycle is only part of a larger value — `resolveValue` filters the + * missing piece out of a descriptor array and keeps the surviving siblings. + */ if (namesBeingResolved.has(name)) { return; } @@ -61,6 +84,12 @@ export function varResolver( namesBeingResolved.add(name); try { + /** + * A name present in the inherited variables resolves to whatever that + * value gives, `fallback` included: an inherited name that resolves to + * nothing swallows `var(--name, blue)`'s fallback rather than using it. + * That holds for any unresolvable inherited value, not just a cyclic one. + */ if (name in variables) { renderGuards?.push(["v", name, variables[name]]); return resolve(variables[name]); From 0c6d40af475bf02359e9d1d1e7b9ea0b57edde12 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Tue, 22 Sep 2026 20:32:06 +0300 Subject: [PATCH 3/6] refactor(native): drop a var lookup that cannot be reached MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `varResolver` re-resolved `variables[name]` after the `name in variables` check had already returned for every name that object holds — so `variables[name]` is necessarily `undefined` there and the branch could not be entered. Proven rather than reasoned: deleting the block leaves the suite byte-identical to this branch's own baseline, 1058 passed and the same three Windows babel failures. Two cases beside it pin what the ladder does once a name is NOT inherited: a universal definition resolves, and an inherited one outranks it. Both carry a second definition of the name, because a single-definition variable is inlined at compile time and never reaches this resolver at all. --- src/__tests__/native/variables.test.tsx | 30 +++++++++++++++++++++++++ src/native/styles/variables.ts | 9 -------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/__tests__/native/variables.test.tsx b/src/__tests__/native/variables.test.tsx index a7a2ccb1..0a5d30d2 100644 --- a/src/__tests__/native/variables.test.tsx +++ b/src/__tests__/native/variables.test.tsx @@ -471,4 +471,34 @@ describe("circular variables", () => { opacity: 0.5, }); }); + + // A name with ONE definition is inlined at compile time, so each of these carries a second the + // element never inherits — that is what leaves the lookup to the runtime at all. + test("a universal variable resolves when no scope above defines the name", () => { + registerCSS(` + * { --universal: 7px; } + .elsewhere { --universal: 99px; } + .uses-universal { width: var(--universal); } + `); + + render(); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ width: 7 }); + }); + + test("an inherited definition outranks the universal one", () => { + registerCSS(` + * { --universal: 7px; } + .defines { --universal: 11px; } + .uses-universal { width: var(--universal); } + `); + + render( + + + , + ); + + expect(screen.getByTestId(testID).props.style).toStrictEqual({ width: 11 }); + }); }); diff --git a/src/native/styles/variables.ts b/src/native/styles/variables.ts index fdfa019a..fcafe7a3 100644 --- a/src/native/styles/variables.ts +++ b/src/native/styles/variables.ts @@ -103,15 +103,6 @@ export function varResolver( return value; } - value = resolve(variables[name]); - if (value !== undefined) { - renderGuards?.push(["v", name, value]); - options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; - options.inlineVariables[name] = value; - - return value; - } - value = resolve(get(universalVariables(name))); if (value !== undefined) { options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; From 4acfcf09c554987f4792c97df3458c3284a78219 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Tue, 22 Sep 2026 21:05:01 +0300 Subject: [PATCH 4/6] revert(native): keep the unreachable var lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit removed the `variables[name]` re-resolve on the grounds that `name in variables` has already returned for every name that object holds, so the branch cannot be entered — deleting it left the suite byte-identical. That argument holds for a plain record and stops holding the moment the inherited scope is anything else. The rung costs one comparison on a path that already ran three, and its absence would be a silent behaviour change rather than a loud one, so it stays and the observation lives in the PR body instead. The two cases added beside it stand: a universal definition resolves when no scope above declares the name, and an inherited one outranks it. --- .pr-body-422.md | 147 +++++++++++++++++++++++++++++++++ src/native/styles/variables.ts | 9 ++ 2 files changed, 156 insertions(+) create mode 100644 .pr-body-422.md diff --git a/.pr-body-422.md b/.pr-body-422.md new file mode 100644 index 00000000..087d0463 --- /dev/null +++ b/.pr-body-422.md @@ -0,0 +1,147 @@ +## Problem + +CSS Variables Level 1 §3 is explicit about reference cycles: *"if there is a cycle in the dependency graph, all the custom properties in the cycle must compute to their guaranteed-invalid value"* — which makes the consuming declaration invalid at computed-value time and leaves the property unset. A well-defined, non-fatal outcome. + +The native runtime instead recurses until the JS stack is exhausted and throws out of `render`. Measured on `main` (`f70c402`) through `@testing-library/react-native`: + +```js +registerCSS(`.themed { --a: var(--b); --b: var(--a); width: var(--a) }`, { inlineVariables: false }); +render(); +``` + +``` +RangeError: Maximum call stack size exceeded + at resolve (src/native/styles/variables.ts:51:12) + at resolveValue (src/native/styles/resolve.ts:105:16) + at resolveValue (src/native/styles/resolve.ts:109:27) + at resolve (src/native/styles/variables.ts:51:12) + … repeating +``` + +In an app that reaches whatever error boundary sits above the tree, so one cyclic custom property replaces a screen with a fallback. + +### Why it is reachable from ordinary CSS + +The compiler has a *working* cycle guard of its own — `flattenVar`'s `seen` set in the `inlineVariables` pass — which is why this is not fired by every stylesheet. But that pass only folds a custom property declared **exactly once** (`src/compiler/inline-variables.ts`: `if (info.count !== 1) vars.delete(name)`), so the same CSS with default options is folded away before the runtime sees it. Declare the token twice — a base value plus a `prefers-color-scheme` override, which is the ordinary shape of themed CSS — and it skips the inliner and reaches the resolver. + +Measured on `main`, all three with the same class: + +| stylesheet | `main` | this branch | +| --- | --- | --- | +| the cycle, `inlineVariables: false` | **throws** | `{}` | +| the cycle, compiler defaults | `{}` — inlined away | `{}` | +| the cycle plus `@media (prefers-color-scheme: dark) { .themed { --a: 10px } }`, **compiler defaults** | **throws** | `{}` | + +The third row is the point: no compiler option changed, no exotic input, just a theme token and a cycle someone did not notice. + +Cycles split across an ancestor/descendant pair reach it too, because inherited variables resolve through the same function — and that is the shape the tests use, since a single-definition variable is folded away before the runtime sees it. + +## Root cause — the guard was dead code + +`varResolver` read its guard out of `options` **with a default**, and never wrote it back: + +```ts +const { + … + variableHistory = new Set(), // ← a fresh Set on every call +} = options; + +if (variableHistory.has(name)) return; // ← therefore always false +variableHistory.add(name); // ← mutates a Set that is discarded +``` + +`resolve` closes over the same `options` object, whose `.variableHistory` stays `undefined`, so every nested `varResolver` allocated a new empty Set. There are exactly four `variableHistory` references in `src/` on `main` — the optional field on `ResolveValueOptions`, and the destructure / `.has` / `.add` above. Nothing anywhere assigns it. + +A second hole sits beside it: the `if (name in variables)` early return recurses **before** `.add` is reached. That is the branch a descendant takes, and therefore the branch the recursion runs through, so even a working set would have been bypassed. + +## Fix + +Two changes, both in `varResolver`. + +1. **The set lives on `options`** — `const namesBeingResolved = (options.namesBeingResolved ??= new Set())` — so every nested resolve below shares it. +2. **It holds the names on the CURRENT path**, not every name ever seen: added before any of the name's values are resolved, removed again in a `finally`. The `name in variables` branch moves inside the `try`. + +**Both halves are load-bearing, and each is pinned from its own side.** Registering the name cuts the cycle. Removing it per frame — rather than emptying the stack — is what keeps a name readable again once its own resolution has finished, which a name read twice within ONE declaration needs (`box-shadow: var(--c) 1px 1px, var(--c) 2px 2px`). + +Only the within-one-declaration case depends on it. `applyDeclarations` builds a fresh options object at each of its three `resolveValue` call sites in `calculate-props.ts` — the transform, delayed and plain arms — so two declarations never share a stack in the first place. Measured: hoisting a single shared options object across every declaration leaves the **whole suite** at the exact baseline, `1058 passed / 3 failed`. So the `finally` is not there to un-block a second declaration; it is there so that one declaration's second read, and one value's second branch, are not mistaken for re-entry. + +The field is renamed `variableHistory` → `namesBeingResolved`, matching the local it feeds and what it holds. `ResolveValueOptions` is internal to `src/native/styles/` and the field is re-exported from no entry point. + +The diff looks larger than it is — a good part of it is the four existing lookup tiers moving one indentation level into the `try`, unchanged. + +## Which plane + +**Native runtime** (`src/native/styles/variables.ts`), alone. `varResolver` is referenced only from `src/native/styles/resolve.ts`, and there is no variable resolution anywhere under `src/web` — on web the CSS is served to the browser and the cycle rule above is the browser's to implement. So there is no web mirror to write. + +## Tests + +10 cases in `src/__tests__/native/variables.test.tsx`, `describe("circular variables")`. **6 of them fail with `RangeError: Maximum call stack size exceeded` against the unfixed `varResolver`** — that is the reproduction. Substituting `main`'s `varResolver` back in under these tests gives `1052 passed, 9 failed` against this branch's `1058 passed, 3 failed` — 6 new reds plus the 3 pre-existing Windows failures below. The 6 are the four census rows and both public entry points. + +- **A four-row census** driven by `test.each` behind a not-empty sentinel: a variable whose value is itself, a variable reached again through a fallback, two variables that name each other, and one name re-entered from two branches of a single value. +- **Both public entry points that recurse without the guard**: `useUnstableNativeVariable`, and `VariableContextProvider`, whose value type admits a `var()` reference. +- **Two guards that the cut is not over-broad**, green on `main` too: a variable read twice in ONE declaration (a two-shadow `box-shadow`) is not mistaken for a cycle, and a long non-circular chain still resolves. +- **The compiler's own cycle guard**, which nothing in the repo covered: `.child { --z: var(--z); width: var(--z) }` with `--z` declared once never reaches the runtime, so `flattenVar`'s `seen` set is what stops it. That guard decides whether the runtime guard is reached at all, and until now it had no test anywhere. + +No test asserts a throw. Every case asserts a successful render, so the crash is proven red-to-green rather than pinned as a `toThrow`. + +### Every row is falsifiable + +A census row asserting only `toStrictEqual({})` cannot tell "the cycle was cut" from "resolution no longer works" — an empty style is what both produce. So every row reads a non-cyclic `--unrelated` beside the cycle, and a row can only pass while resolution still works. Every name in a cycle is also declared twice, because a variable declared exactly once is substituted into its readers and never reaches the runtime resolver these rows exist to exercise. + +Measured by mutating the guard and running the full suite. Baseline is `3 failed` — the pre-existing Windows-only `babel-plugin-tester` cases, below. + +| mutation | red | +| --- | --- | +| `varResolver` returns `undefined` unconditionally — every `var()` in the library dead | **143** suite-wide, including all four census rows | +| the cut returns `resolve(fallback)` instead of nothing | 1 — `a variable reached again through a fallback`, `{ color: "blue", opacity: 0.5 }` against `{ opacity: 0.5 }` | +| the `finally` empties the stack (`clear()`) instead of popping one frame | 1 — the two-branch row, with `RangeError` | +| the `finally` pops nothing | 1 — the two-shadow `box-shadow` | +| `flattenVar`'s `seen` set removed | 1 — the compile-time row | + +The third and fourth rows pin the `finally` from both sides: removing too little reddens the `box-shadow`, removing too much reddens the two-branch row, and **neither mutation alone reaches the other's test.** + +### Suite + +``` +Test Suites: 2 failed, 4 skipped, 53 passed, 55 of 59 total +Tests: 3 failed, 21 skipped, 1058 passed, 1082 total +``` + +`numRuntimeErrorTestSuites: 0`. `main` (`f70c402`) is `1048 passed, 1072 total` on the same machine, so this is +10 tests and no new failures. The 3 are `react-native › plugin › 7`, `react-native-web › plugin › 6` and `› 17` — `babel-plugin-tester` cases over an unrewritten relative `require("../View")`, which fail identically at every ref on Windows. That count is stable on a warm cache; a cold or loaded run adds a tail of first-in-file 5000ms timeouts that are not this branch's either. `yarn typecheck` and `yarn lint` exit 0. + +## KNOWN LIMITS + +**The cut is not always the spec's outcome — sometimes the property keeps a truncated value.** The cut returns `undefined`, and `resolveValue`'s descriptor-array branch filters `undefined` out of the array and keeps the surviving siblings, so a cycle that is only *part* of a larger value leaves the rest behind rather than invalidating the declaration. Measured on this branch: + +| stylesheet | this branch | CSS says | +| --- | --- | --- | +| `--p: 1px var(--p)`; `width: var(--p)` | `{ width: [1] }` | `width` unset | +| `--a: var(--b) var(--c)`, both naming `--a`; `color: var(--a)` | `{ color: [] }` | `color` unset | +| `--t1`/`--t2` cyclic; `transform: translateX(var(--t1))` | `{ transform: [{}] }` | `transform` unset | +| `--bw` cyclic; `border: var(--bw) solid red` | `{ borderStyle: "solid", borderColor: "red" }` | the whole declaration invalid | + +The two-branch census row pins the second of these (`{ color: [], opacity: 0.5 }`) so the shape is at least recorded rather than incidental. Every one of them is a bounded, renderable value instead of a crash, which is the change this PR is claiming; making them *unset* is a separate change to how `resolveValue` treats a missing piece of a descriptor array, and it would move values that have nothing to do with cycles. + +I have not verified whether `color: []` / `width: [1]` / `transform: [{}]` are tolerated or fatal in React Native's own layer — the measurements above are jest, not a device. + +**The guard bounds cycles only.** A non-circular chain resolves to great depth, but a long enough one still exhausts the JS stack and throws `RangeError`. The guard neither helps nor hurts there — a chain never re-enters a name, so it never reaches the cut — and this PR does not claim to fix it. The ceiling is a property of the JS stack rather than a constant this library owns, so I have deliberately not written a number down; the test pins that a long chain resolves, not how long. + +**A cyclic variable swallows its reader's fallback: `var(--cyclic, blue)` yields `{}` where CSS says `blue`.** This is pre-existing and not cycle-specific. `varResolver`'s first arm is presence-keyed — `if (name in variables) return resolve(variables[name])` — so *any* inherited name that is present and resolves to nothing takes the reader's fallback with it. Measured on this branch: a declared-but-unresolvable non-cyclic variable swallows the fallback, a cyclic one swallows it, and a never-declared name correctly takes it. Those are `main`'s results too — this PR moves that arm one indentation level into the `try` and changes nothing else about it. #431 documents this exact shape in its own body, but its fix is at the two record builders that plant a key holding `undefined`; it does not touch that early return, so landing #431 will not fix the CSS-declared case. + +**The keyframes boundary is an open question, not a fix.** `shorthands/animation.ts` re-enters `calculateProps` with a fresh options object, so the resolution stack does not cross into a keyframe pass. Three attempts to construct a cycle that is reachable through it all rendered cleanly — the animation name resolves and its frame pops before the keyframe pass runs, leaving no live frame to re-enter. With no reproduction I have not written a fix; there is a comment marking the boundary so the next person starts from what is known. + +**Nothing warns.** A stylesheet with an accidental cycle silently loses a declaration where it previously lost the screen. That is strictly better, but if you would like a dev-mode warning at the cut point it is a two-line addition and I will add it. + +**The compile-time guard and this one remain two guards.** `flattenVar`'s `seen` set and this resolution stack solve the same problem at different times, and neither knows about the other. Unifying them is not possible as things stand — the compiler can only see cycles inside the properties it is allowed to fold — so this is a note rather than a plan. Both now have a test. + +--- + +**Overlaps with open PRs**, measured with a three-way `git merge-file` of each PR head against the shared base `f70c402`: + +**#412 (`fix/non-inheriting-custom-properties`) conflicts, and the substantive risk is larger than the textual one.** It adds two rungs to `src/native/styles/variables.ts` — a `nonInheritedVariables` gate around the `rootVariables` lookup, and a new `registeredInitialValues` lookup after it — in exactly the region this PR re-indents into its `try`. One conflict hunk, resolvable by hand in a minute. What a hand-merge must get right is that **both of #412's rungs land INSIDE the `try`**, alongside the four existing tiers. Land them after the `finally` and that tier resolves outside the resolution stack, unguarded, with no test on either branch that would notice. + +**#431 (`fix/vars-undefined-key`) conflicts trivially**, in `src/__tests__/native/variables.test.tsx` and nowhere else: both PRs add an import from `react-native-css/native` at the top of the file, this one for `useUnstableNativeVariable` and `VariableContextProvider`, #431 for `VariableContextProvider` alone. One hunk, one merged import statement. + +**#413 (`fix/single-definition-inliner-scope`) and #389 (`fix/scale-percentage`) auto-merge clean today.** #413 shares `src/__tests__/native/variables.test.tsx` and `src/compiler/inline-variables.ts`, #389 shares `src/native/styles/resolve.ts`; all three files merge without a conflict. + +#413 is also related in substance, in a way that helps: it scopes the single-definition inliner to its declaring block, which means *more* custom properties survive to the runtime resolver. Landing it without this one widens the surface on which the crash is reachable. diff --git a/src/native/styles/variables.ts b/src/native/styles/variables.ts index fcafe7a3..fdfa019a 100644 --- a/src/native/styles/variables.ts +++ b/src/native/styles/variables.ts @@ -103,6 +103,15 @@ export function varResolver( return value; } + value = resolve(variables[name]); + if (value !== undefined) { + renderGuards?.push(["v", name, value]); + options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; + options.inlineVariables[name] = value; + + return value; + } + value = resolve(get(universalVariables(name))); if (value !== undefined) { options.inlineVariables ??= { [VAR_SYMBOL]: "inline" }; From 55ad33039334a692529c29bb706ad4a26f6367b9 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Tue, 22 Sep 2026 21:05:29 +0300 Subject: [PATCH 5/6] chore: untrack a scratch PR-body draft --- .pr-body-422.md | 147 ------------------------------------------------ 1 file changed, 147 deletions(-) delete mode 100644 .pr-body-422.md diff --git a/.pr-body-422.md b/.pr-body-422.md deleted file mode 100644 index 087d0463..00000000 --- a/.pr-body-422.md +++ /dev/null @@ -1,147 +0,0 @@ -## Problem - -CSS Variables Level 1 §3 is explicit about reference cycles: *"if there is a cycle in the dependency graph, all the custom properties in the cycle must compute to their guaranteed-invalid value"* — which makes the consuming declaration invalid at computed-value time and leaves the property unset. A well-defined, non-fatal outcome. - -The native runtime instead recurses until the JS stack is exhausted and throws out of `render`. Measured on `main` (`f70c402`) through `@testing-library/react-native`: - -```js -registerCSS(`.themed { --a: var(--b); --b: var(--a); width: var(--a) }`, { inlineVariables: false }); -render(); -``` - -``` -RangeError: Maximum call stack size exceeded - at resolve (src/native/styles/variables.ts:51:12) - at resolveValue (src/native/styles/resolve.ts:105:16) - at resolveValue (src/native/styles/resolve.ts:109:27) - at resolve (src/native/styles/variables.ts:51:12) - … repeating -``` - -In an app that reaches whatever error boundary sits above the tree, so one cyclic custom property replaces a screen with a fallback. - -### Why it is reachable from ordinary CSS - -The compiler has a *working* cycle guard of its own — `flattenVar`'s `seen` set in the `inlineVariables` pass — which is why this is not fired by every stylesheet. But that pass only folds a custom property declared **exactly once** (`src/compiler/inline-variables.ts`: `if (info.count !== 1) vars.delete(name)`), so the same CSS with default options is folded away before the runtime sees it. Declare the token twice — a base value plus a `prefers-color-scheme` override, which is the ordinary shape of themed CSS — and it skips the inliner and reaches the resolver. - -Measured on `main`, all three with the same class: - -| stylesheet | `main` | this branch | -| --- | --- | --- | -| the cycle, `inlineVariables: false` | **throws** | `{}` | -| the cycle, compiler defaults | `{}` — inlined away | `{}` | -| the cycle plus `@media (prefers-color-scheme: dark) { .themed { --a: 10px } }`, **compiler defaults** | **throws** | `{}` | - -The third row is the point: no compiler option changed, no exotic input, just a theme token and a cycle someone did not notice. - -Cycles split across an ancestor/descendant pair reach it too, because inherited variables resolve through the same function — and that is the shape the tests use, since a single-definition variable is folded away before the runtime sees it. - -## Root cause — the guard was dead code - -`varResolver` read its guard out of `options` **with a default**, and never wrote it back: - -```ts -const { - … - variableHistory = new Set(), // ← a fresh Set on every call -} = options; - -if (variableHistory.has(name)) return; // ← therefore always false -variableHistory.add(name); // ← mutates a Set that is discarded -``` - -`resolve` closes over the same `options` object, whose `.variableHistory` stays `undefined`, so every nested `varResolver` allocated a new empty Set. There are exactly four `variableHistory` references in `src/` on `main` — the optional field on `ResolveValueOptions`, and the destructure / `.has` / `.add` above. Nothing anywhere assigns it. - -A second hole sits beside it: the `if (name in variables)` early return recurses **before** `.add` is reached. That is the branch a descendant takes, and therefore the branch the recursion runs through, so even a working set would have been bypassed. - -## Fix - -Two changes, both in `varResolver`. - -1. **The set lives on `options`** — `const namesBeingResolved = (options.namesBeingResolved ??= new Set())` — so every nested resolve below shares it. -2. **It holds the names on the CURRENT path**, not every name ever seen: added before any of the name's values are resolved, removed again in a `finally`. The `name in variables` branch moves inside the `try`. - -**Both halves are load-bearing, and each is pinned from its own side.** Registering the name cuts the cycle. Removing it per frame — rather than emptying the stack — is what keeps a name readable again once its own resolution has finished, which a name read twice within ONE declaration needs (`box-shadow: var(--c) 1px 1px, var(--c) 2px 2px`). - -Only the within-one-declaration case depends on it. `applyDeclarations` builds a fresh options object at each of its three `resolveValue` call sites in `calculate-props.ts` — the transform, delayed and plain arms — so two declarations never share a stack in the first place. Measured: hoisting a single shared options object across every declaration leaves the **whole suite** at the exact baseline, `1058 passed / 3 failed`. So the `finally` is not there to un-block a second declaration; it is there so that one declaration's second read, and one value's second branch, are not mistaken for re-entry. - -The field is renamed `variableHistory` → `namesBeingResolved`, matching the local it feeds and what it holds. `ResolveValueOptions` is internal to `src/native/styles/` and the field is re-exported from no entry point. - -The diff looks larger than it is — a good part of it is the four existing lookup tiers moving one indentation level into the `try`, unchanged. - -## Which plane - -**Native runtime** (`src/native/styles/variables.ts`), alone. `varResolver` is referenced only from `src/native/styles/resolve.ts`, and there is no variable resolution anywhere under `src/web` — on web the CSS is served to the browser and the cycle rule above is the browser's to implement. So there is no web mirror to write. - -## Tests - -10 cases in `src/__tests__/native/variables.test.tsx`, `describe("circular variables")`. **6 of them fail with `RangeError: Maximum call stack size exceeded` against the unfixed `varResolver`** — that is the reproduction. Substituting `main`'s `varResolver` back in under these tests gives `1052 passed, 9 failed` against this branch's `1058 passed, 3 failed` — 6 new reds plus the 3 pre-existing Windows failures below. The 6 are the four census rows and both public entry points. - -- **A four-row census** driven by `test.each` behind a not-empty sentinel: a variable whose value is itself, a variable reached again through a fallback, two variables that name each other, and one name re-entered from two branches of a single value. -- **Both public entry points that recurse without the guard**: `useUnstableNativeVariable`, and `VariableContextProvider`, whose value type admits a `var()` reference. -- **Two guards that the cut is not over-broad**, green on `main` too: a variable read twice in ONE declaration (a two-shadow `box-shadow`) is not mistaken for a cycle, and a long non-circular chain still resolves. -- **The compiler's own cycle guard**, which nothing in the repo covered: `.child { --z: var(--z); width: var(--z) }` with `--z` declared once never reaches the runtime, so `flattenVar`'s `seen` set is what stops it. That guard decides whether the runtime guard is reached at all, and until now it had no test anywhere. - -No test asserts a throw. Every case asserts a successful render, so the crash is proven red-to-green rather than pinned as a `toThrow`. - -### Every row is falsifiable - -A census row asserting only `toStrictEqual({})` cannot tell "the cycle was cut" from "resolution no longer works" — an empty style is what both produce. So every row reads a non-cyclic `--unrelated` beside the cycle, and a row can only pass while resolution still works. Every name in a cycle is also declared twice, because a variable declared exactly once is substituted into its readers and never reaches the runtime resolver these rows exist to exercise. - -Measured by mutating the guard and running the full suite. Baseline is `3 failed` — the pre-existing Windows-only `babel-plugin-tester` cases, below. - -| mutation | red | -| --- | --- | -| `varResolver` returns `undefined` unconditionally — every `var()` in the library dead | **143** suite-wide, including all four census rows | -| the cut returns `resolve(fallback)` instead of nothing | 1 — `a variable reached again through a fallback`, `{ color: "blue", opacity: 0.5 }` against `{ opacity: 0.5 }` | -| the `finally` empties the stack (`clear()`) instead of popping one frame | 1 — the two-branch row, with `RangeError` | -| the `finally` pops nothing | 1 — the two-shadow `box-shadow` | -| `flattenVar`'s `seen` set removed | 1 — the compile-time row | - -The third and fourth rows pin the `finally` from both sides: removing too little reddens the `box-shadow`, removing too much reddens the two-branch row, and **neither mutation alone reaches the other's test.** - -### Suite - -``` -Test Suites: 2 failed, 4 skipped, 53 passed, 55 of 59 total -Tests: 3 failed, 21 skipped, 1058 passed, 1082 total -``` - -`numRuntimeErrorTestSuites: 0`. `main` (`f70c402`) is `1048 passed, 1072 total` on the same machine, so this is +10 tests and no new failures. The 3 are `react-native › plugin › 7`, `react-native-web › plugin › 6` and `› 17` — `babel-plugin-tester` cases over an unrewritten relative `require("../View")`, which fail identically at every ref on Windows. That count is stable on a warm cache; a cold or loaded run adds a tail of first-in-file 5000ms timeouts that are not this branch's either. `yarn typecheck` and `yarn lint` exit 0. - -## KNOWN LIMITS - -**The cut is not always the spec's outcome — sometimes the property keeps a truncated value.** The cut returns `undefined`, and `resolveValue`'s descriptor-array branch filters `undefined` out of the array and keeps the surviving siblings, so a cycle that is only *part* of a larger value leaves the rest behind rather than invalidating the declaration. Measured on this branch: - -| stylesheet | this branch | CSS says | -| --- | --- | --- | -| `--p: 1px var(--p)`; `width: var(--p)` | `{ width: [1] }` | `width` unset | -| `--a: var(--b) var(--c)`, both naming `--a`; `color: var(--a)` | `{ color: [] }` | `color` unset | -| `--t1`/`--t2` cyclic; `transform: translateX(var(--t1))` | `{ transform: [{}] }` | `transform` unset | -| `--bw` cyclic; `border: var(--bw) solid red` | `{ borderStyle: "solid", borderColor: "red" }` | the whole declaration invalid | - -The two-branch census row pins the second of these (`{ color: [], opacity: 0.5 }`) so the shape is at least recorded rather than incidental. Every one of them is a bounded, renderable value instead of a crash, which is the change this PR is claiming; making them *unset* is a separate change to how `resolveValue` treats a missing piece of a descriptor array, and it would move values that have nothing to do with cycles. - -I have not verified whether `color: []` / `width: [1]` / `transform: [{}]` are tolerated or fatal in React Native's own layer — the measurements above are jest, not a device. - -**The guard bounds cycles only.** A non-circular chain resolves to great depth, but a long enough one still exhausts the JS stack and throws `RangeError`. The guard neither helps nor hurts there — a chain never re-enters a name, so it never reaches the cut — and this PR does not claim to fix it. The ceiling is a property of the JS stack rather than a constant this library owns, so I have deliberately not written a number down; the test pins that a long chain resolves, not how long. - -**A cyclic variable swallows its reader's fallback: `var(--cyclic, blue)` yields `{}` where CSS says `blue`.** This is pre-existing and not cycle-specific. `varResolver`'s first arm is presence-keyed — `if (name in variables) return resolve(variables[name])` — so *any* inherited name that is present and resolves to nothing takes the reader's fallback with it. Measured on this branch: a declared-but-unresolvable non-cyclic variable swallows the fallback, a cyclic one swallows it, and a never-declared name correctly takes it. Those are `main`'s results too — this PR moves that arm one indentation level into the `try` and changes nothing else about it. #431 documents this exact shape in its own body, but its fix is at the two record builders that plant a key holding `undefined`; it does not touch that early return, so landing #431 will not fix the CSS-declared case. - -**The keyframes boundary is an open question, not a fix.** `shorthands/animation.ts` re-enters `calculateProps` with a fresh options object, so the resolution stack does not cross into a keyframe pass. Three attempts to construct a cycle that is reachable through it all rendered cleanly — the animation name resolves and its frame pops before the keyframe pass runs, leaving no live frame to re-enter. With no reproduction I have not written a fix; there is a comment marking the boundary so the next person starts from what is known. - -**Nothing warns.** A stylesheet with an accidental cycle silently loses a declaration where it previously lost the screen. That is strictly better, but if you would like a dev-mode warning at the cut point it is a two-line addition and I will add it. - -**The compile-time guard and this one remain two guards.** `flattenVar`'s `seen` set and this resolution stack solve the same problem at different times, and neither knows about the other. Unifying them is not possible as things stand — the compiler can only see cycles inside the properties it is allowed to fold — so this is a note rather than a plan. Both now have a test. - ---- - -**Overlaps with open PRs**, measured with a three-way `git merge-file` of each PR head against the shared base `f70c402`: - -**#412 (`fix/non-inheriting-custom-properties`) conflicts, and the substantive risk is larger than the textual one.** It adds two rungs to `src/native/styles/variables.ts` — a `nonInheritedVariables` gate around the `rootVariables` lookup, and a new `registeredInitialValues` lookup after it — in exactly the region this PR re-indents into its `try`. One conflict hunk, resolvable by hand in a minute. What a hand-merge must get right is that **both of #412's rungs land INSIDE the `try`**, alongside the four existing tiers. Land them after the `finally` and that tier resolves outside the resolution stack, unguarded, with no test on either branch that would notice. - -**#431 (`fix/vars-undefined-key`) conflicts trivially**, in `src/__tests__/native/variables.test.tsx` and nowhere else: both PRs add an import from `react-native-css/native` at the top of the file, this one for `useUnstableNativeVariable` and `VariableContextProvider`, #431 for `VariableContextProvider` alone. One hunk, one merged import statement. - -**#413 (`fix/single-definition-inliner-scope`) and #389 (`fix/scale-percentage`) auto-merge clean today.** #413 shares `src/__tests__/native/variables.test.tsx` and `src/compiler/inline-variables.ts`, #389 shares `src/native/styles/resolve.ts`; all three files merge without a conflict. - -#413 is also related in substance, in a way that helps: it scopes the single-definition inliner to its declaring block, which means *more* custom properties survive to the runtime resolver. Landing it without this one widens the surface on which the crash is reachable. From 3ab7d96870add82f58a2222c32a5407d90017575 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Wed, 23 Sep 2026 02:09:14 +0300 Subject: [PATCH 6/6] test(compiler): pin the cycle shape the runtime guard is handed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This branch's compiler file carries a docblock and no code, so these six cases pass on main — and that is the claim rather than a gap. The runtime guard added here only ever runs because the compiler hands it an UNSUBSTITUTED reference; a compiler that started inlining a cycle, or throwing on one, would make the guard unreachable and no suite would say so. Three cycle shapes assert the reference survives compilation, and two acyclic cases assert substitution still happens, so the set cannot pass by the compiler simply giving up on every variable. --- .../compiler/variable-cycles.test.ts | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/__tests__/compiler/variable-cycles.test.ts diff --git a/src/__tests__/compiler/variable-cycles.test.ts b/src/__tests__/compiler/variable-cycles.test.ts new file mode 100644 index 00000000..0da630e7 --- /dev/null +++ b/src/__tests__/compiler/variable-cycles.test.ts @@ -0,0 +1,61 @@ +import { compile } from "react-native-css/compiler"; + +/** + * Substitution follows the same references the runtime resolver does, so a + * self-referential value has no base case at compile time either. The runtime + * suite exercises the resolver's own guard; this covers the compiler's, which + * runs first and would otherwise never hand it anything. + */ +function firstDeclaration(css: string, className: string): unknown { + const entry = (compile(css).stylesheet().s ?? []).find( + ([name]) => name === className, + ); + if (!entry) { + throw new Error(`the compiler emitted no rule for .${className}`); + } + return entry[1][0]?.d; +} + +const CYCLES = [ + [ + "a variable referencing itself", + `.t { --x: var(--x); color: var(--x) }`, + "x", + ], + [ + "two variables referencing each other", + `.t { --p: var(--q); --q: var(--p); color: var(--p) }`, + "p", + ], + [ + "a three-step cycle", + `.t { --a: var(--b); --b: var(--c); --c: var(--a); color: var(--a) }`, + "a", + ], +] as const; + +describe("a variable cycle is left for the runtime rather than inlined", () => { + test("the census is not empty, so the cases below are not vacuous", () => { + expect(CYCLES.length).toBeGreaterThan(0); + }); + + test.each(CYCLES)("%s", (_label, css, variableName) => { + expect(firstDeclaration(css, "t")).toStrictEqual([ + [[{}, "var", variableName, 1], "color", 1], + ]); + }); +}); + +describe("a variable with no cycle is still inlined", () => { + test("a direct value substitutes", () => { + expect( + firstDeclaration(`.t { --n: red; color: var(--n) }`, "t"), + ).toStrictEqual([{ color: "#f00" }]); + }); + + test("a chain substitutes through every step", () => { + expect( + firstDeclaration(`.t { --a: red; --b: var(--a); color: var(--b) }`, "t"), + ).toStrictEqual([{ color: "#f00" }]); + }); +});