Skip to content

Commit adc1691

Browse files
fix(native): a stylesheet reload retracts a registered initial value
Deleting an `@property` rule un-registered half of it. `inject` clears `nonInheritedVariables` before re-adding, so the name stopped being non-inheriting, but nothing cleared `registeredInitialValues` before the `options.vi` loop. The initial value outlived the rule that declared it, and the two halves of one registration disagreed for the rest of the session. Measured on one sheet, `.probe { width: calc(2px + var(--my-var)) }` against an `initial-value: 3px` registration: cold, no @Property rule style {} @Property present, then deleted style { width: 5 } Retracting is not a `clear()`. `family.clear()` is a `Map.clear()` and notifies nobody: a mounted element keeps painting the deleted value, and the next registration of that name lands on a fresh observable that element never subscribed to. Measured — with `clear()` in `inject` the registry reads `undefined` while the element still paints `width: 5`, so the registry-level assertion alone would bless it. `set(undefined)` takes the same notification path a changed value takes and leaves the observable its readers hold in place. That needs the names the previous sheet registered, so `family` gains `keys()`. Its Map is already that record; a second registry alongside it would be the same two-halves-disagreeing shape as the defect. The observable's argument widens to accept `undefined`, which the read function's first line already implements. `resetVariableRegistries` keeps clearing, because nothing is mounted across the test boundary it serves and it has no reader to strand. Its clear of `registeredInitialValues` was unguarded — removing the line left all 1121 tests green — and now turns exactly one red. Correcting the record from 304a506: `root.ts` holds four variable stores and a second copy of the module cannot reach three of them. That commit pins one, `registeredInitialValues`. `rootVariables` and `universalVariables` are pre-existing and are PR 410's subject; `nonInheritedVariables`, the other store this branch adds, was already pinned. Five tests. Four are runtime, because only a mounted reader separates a retraction that notifies from one that does not: the cold-load control, the registry retraction, the mounted element, and an observable identity that fails the moment the retraction becomes a `clear()`. The fifth pins the sheet a deleted rule compiles to, which is what the runtime retracts against.
1 parent 304a506 commit adc1691

5 files changed

Lines changed: 168 additions & 17 deletions

File tree

‎src/__tests__/compiler/property.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,9 @@ test("an unregistered custom property is not recorded", () => {
359359
const result = compiled.stylesheet();
360360
expect(result.s).toBeDefined();
361361
expect(result.vn).toBeUndefined();
362+
// Neither half of a registration is emitted. This is the sheet a Fast Refresh produces
363+
// when an @property rule is deleted, so it is what the runtime has to retract AGAINST
364+
expect(result.vi).toBeUndefined();
362365
});
363366

364367
test("@property records a name once, however many rules declare it", () => {

‎src/__tests__/native/non-inheriting-channels.test.tsx‎

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
1-
import { render, screen } from "@testing-library/react-native";
1+
import { act, render, screen } from "@testing-library/react-native";
22
import { VariableContextProvider } from "react-native-css";
33
import { View } from "react-native-css/components/View";
44
import { registerCSS, testID } from "react-native-css/jest";
5-
import { nonInheritedVariables } from "react-native-css/native-internal";
5+
import {
6+
nonInheritedVariables,
7+
registeredInitialValues,
8+
resetVariableRegistries,
9+
} from "react-native-css/native-internal";
610

711
const parentTestID = "parent";
812

@@ -342,6 +346,102 @@ test("re-registering keeps the registry object identity", () => {
342346
expect(globalThis.__react_native_css_non_inherited_variables).toBe(before);
343347
});
344348

349+
/* ------------------------------------------------------------------ *
350+
* Lifecycle — a reload retracts a registered initial value
351+
* ------------------------------------------------------------------ */
352+
353+
// Composed into arithmetic ON THE DECLARING ELEMENT, which is how Tailwind reads
354+
// `--tw-ring-offset-width`: `calc(2px + var(--tw-ring-offset-width))`. A registration that
355+
// outlives the rule declaring it does not hand a descendant something it should not have
356+
// inherited, it corrupts a length an element computes for itself
357+
const initialValueRegistration = `
358+
@property --my-var {
359+
syntax: "<length>";
360+
inherits: false;
361+
initial-value: 3px;
362+
}
363+
`;
364+
365+
const initialValueConsumer = `
366+
.probe { width: calc(2px + var(--my-var)); }
367+
.a { --my-var: 10px; }
368+
.b { --my-var: 20px; }
369+
`;
370+
371+
test("a sheet registering nothing leaves the consumer no width", () => {
372+
// The control for the two tests below. `.probe` reads a property no rule it matches
373+
// declares and no @property registers, so the whole declaration drops
374+
registerCSS(initialValueConsumer);
375+
376+
render(<View testID={testID} className="probe" />);
377+
378+
expect(screen.getByTestId(testID).props.style).toStrictEqual({});
379+
});
380+
381+
test("deleting an @property rule retracts its initial value", () => {
382+
registerCSS(`
383+
${initialValueRegistration}
384+
${initialValueConsumer}
385+
`);
386+
expect(registeredInitialValues("my-var").get()).toBe(3);
387+
388+
registerCSS(initialValueConsumer);
389+
390+
expect(registeredInitialValues("my-var").get()).toBeUndefined();
391+
});
392+
393+
test("a mounted element drops a retracted initial value", () => {
394+
// Deleting an @property rule un-registered only half of it: the name left
395+
// `nonInheritedVariables` and the initial value stayed, so the two halves of one
396+
// registration disagreed and the element kept painting a width the sheet no longer
397+
// declares anywhere
398+
registerCSS(`
399+
${initialValueRegistration}
400+
${initialValueConsumer}
401+
`);
402+
403+
render(<View testID={testID} className="probe" />);
404+
expect(screen.getByTestId(testID).props.style).toStrictEqual({ width: 5 });
405+
406+
act(() => {
407+
registerCSS(initialValueConsumer);
408+
});
409+
410+
// Exactly what the same sheet paints when it is the first one loaded, two tests above
411+
expect(screen.getByTestId(testID).props.style).toStrictEqual({});
412+
});
413+
414+
test("retracting an initial value keeps the observable its readers hold", () => {
415+
// This is why the retraction is `.set(undefined)` and not `.clear()`. Clearing the family
416+
// drops the map entry without notifying anyone, so a mounted reader keeps the deleted
417+
// value AND the next registration of the same name lands on an observable it never
418+
// subscribed to — the reader is then stranded for the rest of the session
419+
registerCSS(`
420+
${initialValueRegistration}
421+
${initialValueConsumer}
422+
`);
423+
const before = registeredInitialValues("my-var");
424+
425+
registerCSS(initialValueConsumer);
426+
427+
expect(registeredInitialValues("my-var")).toBe(before);
428+
});
429+
430+
test("resetVariableRegistries retracts a registered initial value", () => {
431+
// The jest preset's beforeEach is all that stands between one test's @property
432+
// registration and the next test's. A plain clear() is right HERE and wrong in inject():
433+
// testing-library unmounts between tests, so this retraction has no reader to strand
434+
registerCSS(`
435+
${initialValueRegistration}
436+
${initialValueConsumer}
437+
`);
438+
expect(registeredInitialValues("my-var").get()).toBe(3);
439+
440+
resetVariableRegistries();
441+
442+
expect(registeredInitialValues("my-var").get()).toBeUndefined();
443+
});
444+
345445
/* ------------------------------------------------------------------ *
346446
* Tailwind v4 ring composition, end to end
347447
* ------------------------------------------------------------------ */

‎src/native-internal/root.ts‎

Lines changed: 50 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
import { Platform, PlatformColor } from "react-native";
22

3-
import type { StyleDescriptor, VariableValue } from "react-native-css/compiler";
3+
import type {
4+
RootVariables,
5+
StyleDescriptor,
6+
VariableValue,
7+
} from "react-native-css/compiler";
48

59
import { testMediaQuery } from "../native/conditions/media-query";
610
import { family, observable, type Observable } from "../native/reactivity";
711

12+
// The argument is nullable because a reload has to be able to RETRACT a name, and the read
13+
// below already answers `undefined` for one — see replaceRegisteredInitialValues
14+
type VariableArg = VariableValue[] | undefined;
15+
816
const rootVariableFamily = () => {
9-
return family<string, Observable<StyleDescriptor, VariableValue[]>>(() => {
10-
const obs = observable<StyleDescriptor, VariableValue[]>(
17+
return family<string, Observable<StyleDescriptor, VariableArg>>(() => {
18+
const obs = observable<StyleDescriptor, VariableArg>(
1119
(read, variableValue) => {
1220
if (!variableValue) return undefined;
1321

@@ -89,6 +97,37 @@ export function assignInheritedVariables<TValue>(
8997
}
9098
}
9199

100+
/**
101+
* Replace every registered initial value with the ones a stylesheet carries.
102+
*
103+
* A reload has to be able to DELETE an `@property` rule, and this registry is observable,
104+
* so dropping the entry is not enough. `family.clear()` is a `Map.clear()`, which notifies
105+
* nobody: a mounted element keeps painting the deleted registration's value, and the next
106+
* registration of that name lands on a fresh observable that element never subscribed to.
107+
* Retracting through `set(undefined)` takes the same notification path a changed value
108+
* takes, and leaves the observable its readers already hold in place.
109+
*
110+
* `resetVariableRegistries` below still clears, because nothing is mounted across the test
111+
* boundary it serves — the mechanism differs where the readers do.
112+
*/
113+
export function replaceRegisteredInitialValues(entries: RootVariables = []) {
114+
const registered = new Set(entries.map(([name]) => name));
115+
116+
// Snapshotted because the family creates an entry for every name the resolver LOOKS UP,
117+
// so a retraction outside a batch can notify a reader that resolves a new one mid-walk.
118+
// Retracting a name that carries no registration is already a no-op: the observable
119+
// recomputes to the `undefined` it holds, compares equal, and notifies nobody
120+
for (const name of Array.from(registeredInitialValues.keys())) {
121+
if (!registered.has(name)) {
122+
registeredInitialValues(name).set(undefined);
123+
}
124+
}
125+
126+
for (const [name, value] of entries) {
127+
registeredInitialValues(name).set(value);
128+
}
129+
}
130+
92131
function seedRootVariables() {
93132
rootVariables("__rn-css-rem").set([[14]]);
94133
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
@@ -107,9 +146,14 @@ seedRootVariables();
107146
/**
108147
* Return every variable registry to its boot state, seeds included.
109148
*
110-
* A stylesheet reload only overwrites the names the new sheet mentions, so a name it
111-
* drops keeps the value the previous one gave it. That is what a reload should do to a
112-
* running app and the opposite of what one test should do to the next.
149+
* A reload replaces the two registries an `@property` rule writes, but `:root` and `*`
150+
* declarations are overwrite-only — a name the new sheet drops keeps the value the previous
151+
* one gave it. That is what a reload should do to a running app and the opposite of what one
152+
* test should do to the next, and a test that injects no stylesheet at all needs the reset
153+
* either way.
154+
*
155+
* Clearing is enough here, where `inject` has to retract through `set(undefined)`: nothing
156+
* is mounted across the boundary this serves, so there is no reader to strand.
113157
*/
114158
export function resetVariableRegistries() {
115159
rootVariables.clear();

‎src/native-internal/style-collection.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import {
1818
nonInheritedVariables,
1919
registeredInitialValues,
20+
replaceRegisteredInitialValues,
2021
rootVariables,
2122
universalVariables,
2223
} from "./root";
@@ -107,16 +108,16 @@ globalThis.__react_native_css_style_collection ??= {
107108
}
108109
}
109110

110-
if (options.vi) {
111-
for (const entry of options.vi) {
112-
registeredInitialValues(entry[0]).set(entry[1]);
113-
}
114-
}
115-
116111
// A stylesheet reload REPLACES the registrations it carries — editing an @property
117-
// rule to `inherits: true`, or deleting it, has to take effect. The container itself
118-
// is kept, because the globalThis pin exists so a second copy of root.ts shares this
119-
// exact Set; swapping it would leave that copy reading one nothing writes to
112+
// rule to `inherits: true`, or deleting it, has to take effect. Both halves of a
113+
// registration are replaced, or the two disagree: the name leaves the Set below while
114+
// its initial value stays behind, and an element goes on painting a length no rule in
115+
// the sheet declares. Retracting an observed value is not a clear() — see root.ts
116+
replaceRegisteredInitialValues(options.vi);
117+
118+
// The container itself is kept, because the globalThis pin exists so a second copy of
119+
// root.ts shares this exact Set; swapping it would leave that copy reading one nothing
120+
// writes to
120121
nonInheritedVariables.clear();
121122

122123
if (options.vn) {

‎src/native/reactivity.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,9 @@ export function family<Key, Result = Key, Args extends any = void>(
140140
delete(key: Key) {
141141
return map.delete(key);
142142
},
143+
keys() {
144+
return map.keys();
145+
},
143146
clear() {
144147
return map.clear();
145148
},

0 commit comments

Comments
 (0)