From 152b5ad6d669b1e248cddc34a57c90f4db2e8aae Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Wed, 19 Aug 2026 20:12:57 +0300 Subject: [PATCH 1/7] fix(native): one resolved-style cache key per distinct input set MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `generateStateHash` keys the resolved-style cache on weak-key identity, so it shares only when equal inputs arrive as one object. Three defects break that. **The config is minted per instance.** `styled()` derives it once at module scope; `useCssElement` derives one per instance from a module constant — same value, fresh identity, so N identical elements each hold their own entry, sorted rule array and observable. `getRuleVariation`'s rule clone is keyed on that identity too, so it was per element rather than per mapping. Deriving through `weakFamily` keys the config on the mapping. A mapping that is not an object is refused by name rather than through the `Invalid value used as weak map key` a primitive would otherwise raise from inside `reactivity`; the predicate is narrower than that WeakMap contract, since a function is a valid weak key and still not a mapping. **`family` and `weakFamily` cache on truthiness.** A factory returning `0` is re-run on every lookup, and `hashKeyFamily` hands out `hashKeyCount++` — so the first weak key hashed never settles. Of twelve call sites it is the only one whose result can be falsy, so caching on presence changes nothing else. **The key is lossy.** Folding N numbers into one and printing it base-36 is a digest, and `family` returns the entry it holds while ignoring the rules the second caller brought — so a collision renders one element's styles on another. Over one config plus one rule, the commonest shape, the fold collides 31.99% of the time. Sorting and joining is exact and keeps order-independence. The numbers are collected into a `Float64Array` so the sort is native: ordering them with `Array.prototype.sort` needs a comparator, and that callback is most of what ordering costs on Hermes — 1.10x the fold at the median key count against 1.64x. Float64 rather than Int32 because the key counter is unbounded and an int32 wraps silently at 2^31. Two inherited lines go with the fold: `generateStateHash`'s empty-string sentinel — unreachable, and collidable once the key is a join — together with the two parameters no caller passes, and the loop's `if (!key) continue`, which erased a member from a value whose whole point is that it erases nothing. `family` gains `size()` so the invariant is assertable. It reaches `rootVariables` and `universalVariables`, which are `family` instances published through `./native-internal` — additive, beside the `delete` and `clear` already on them. 50 identical elements: 50 entries before, 1 after. A 150-control screen on a physical Android device: 1785 → 270-271. Re-entering a screen grew the cache without limit (2, 4, 6 …) and is now flat; entries are still never released on unmount, which is a separate defect. 19 tests across five files, each red first. --- src/__tests__/native/config-identity.test.ts | 121 ++++++++++++++++++ src/__tests__/native/family.test.ts | 42 ++++++ src/__tests__/native/state-hash.test.ts | 73 +++++++++++ .../native/style-cache-bounds.test.tsx | 104 +++++++++++++++ .../native/style-cache-sharing.test.tsx | 43 +++++++ src/native/react/rules.ts | 67 +++++----- src/native/react/useNativeCss.ts | 39 +++++- src/native/reactivity.ts | 7 +- 8 files changed, 459 insertions(+), 37 deletions(-) create mode 100644 src/__tests__/native/config-identity.test.ts create mode 100644 src/__tests__/native/family.test.ts create mode 100644 src/__tests__/native/state-hash.test.ts create mode 100644 src/__tests__/native/style-cache-bounds.test.tsx create mode 100644 src/__tests__/native/style-cache-sharing.test.tsx diff --git a/src/__tests__/native/config-identity.test.ts b/src/__tests__/native/config-identity.test.ts new file mode 100644 index 00000000..703aae80 --- /dev/null +++ b/src/__tests__/native/config-identity.test.ts @@ -0,0 +1,121 @@ +import { ScrollView as RNScrollView, View as RNView } from "react-native"; + +import { generateStateHash } from "../../native/react/rules"; +import { + mappingToConfig, + type ComponentState, + type Config, +} from "../../native/react/useNativeCss"; +import { + VAR_SYMBOL, + type Effect, + type VariableContextValue, +} from "../../native/reactivity"; +import type { StyledConfiguration } from "../../runtime.types"; + +/** + * The resolved-style cache is keyed by `generateStateHash`, which hashes `state.configs` by object + * identity. Sharing therefore depends on one mapping producing one config array, rather than an + * equal array per consumer. + * + * The rules half of that key already holds: `StyleCollection.styles(className)` is keyed by the + * class string, so every element carrying the same class list references one rule set. The config + * half is the one input that is minted per consumer. + */ + +/** A stand-in for the rule set a shared class list resolves to — any object is a valid weak key. */ +const ruleFor = (): WeakKey => ({}); + +/** The smallest `ComponentState` `generateStateHash` reads: it only touches `configs`. */ +const stateFor = (configs: Config[]): ComponentState => { + const observers = new Set(); + const ruleEffect: Effect = { observers, run: () => undefined }; + const inheritedVariables: VariableContextValue = { [VAR_SYMBOL]: true }; + + return { + configs, + inheritedContainers: {}, + inheritedVariables, + ruleEffect, + ruleEffectGetter: (observable) => observable.get(ruleEffect), + styleEffect: { observers, run: () => undefined }, + }; +}; + +const viewMapping = { + className: "style", +} satisfies StyledConfiguration; + +const scrollViewMapping = { + className: "style", + contentContainerClassName: "contentContainerStyle", +} satisfies StyledConfiguration; + +test("two consumers of one mapping share a cache key", () => { + const rules = [ruleFor()]; + + const first = generateStateHash( + stateFor(mappingToConfig(viewMapping)), + rules, + ); + const second = generateStateHash( + stateFor(mappingToConfig(viewMapping)), + rules, + ); + + expect(first).toBe(second); +}); + +test("mappings that differ keep different cache keys", () => { + const rules = [ruleFor()]; + + const view = generateStateHash(stateFor(mappingToConfig(viewMapping)), rules); + const scrollView = generateStateHash( + stateFor(mappingToConfig(scrollViewMapping)), + rules, + ); + + expect(view).not.toBe(scrollView); +}); + +test("a state hash always carries the config, so it is never the empty string", () => { + // The empty string used to double as a no-keys sentinel in `generateStateHash`. With the key a + // join rather than a digest, an empty key list renders as the empty string too — so the sentinel + // and a real state would have shared one cache entry. The config is an unconditional key, which + // is what makes the sentinel unnecessary rather than merely unlikely. + const state = stateFor(mappingToConfig(viewMapping)); + + expect(generateStateHash(state, [])).not.toBe(""); + expect(generateStateHash(state, [ruleFor()])).not.toBe(""); +}); + +test("a mapping that is not an object is refused by name", () => { + // The derivation is cached on the mapping OBJECT. A primitive cannot be a weak-map key, so + // without this guard the failure surfaces as a `WeakMap` error naming nothing the caller wrote. + expect(() => mappingToConfig("style" as never)).toThrow( + /mapping must be an object/u, + ); + expect(() => mappingToConfig(undefined as never)).toThrow( + /mapping must be an object/u, + ); +}); + +test("a mapping mutated after its first use is not re-derived", () => { + // Documented rather than defended: `useCssElement` already froze the derivation per instance, so + // a mutation only ever reached NEWLY mounted elements — the same mapping meaning two things at + // once. Deriving once per mapping settles it on one. + const mapping: Record = { className: "style" }; + const first = mappingToConfig(mapping); + + mapping.className = "contentContainerStyle"; + + expect(mappingToConfig(mapping)).toBe(first); +}); + +test("a mapping built per call still produces an equal config", () => { + // Every wrapper the library ships passes a module constant, but a caller may build the mapping + // inline. That path cannot share on identity and has to keep working unchanged. + expect(mappingToConfig({ className: "style" })).toEqual( + mappingToConfig(viewMapping), + ); +}); diff --git a/src/__tests__/native/family.test.ts b/src/__tests__/native/family.test.ts new file mode 100644 index 00000000..a1022987 --- /dev/null +++ b/src/__tests__/native/family.test.ts @@ -0,0 +1,42 @@ +import { generateHash } from "../../native/react/rules"; +import { family, weakFamily } from "../../native/reactivity"; + +/** + * `family` and `weakFamily` promise one factory call per key, and cached on the result being + * truthy. A factory that legitimately returns `0`, `""` or `false` was therefore re-run on every + * lookup, and its key never settled on a value. + * + * `hashKeyFamily` in `native/react/rules.ts` is such a factory: it hands out `hashKeyCount++`, so + * the first weak key ever hashed is assigned `0` and is the one key that never caches. Its hash + * changes between lookups, which splits every cache keyed on that hash. + */ + +test("weakFamily calls its factory once per key when the result is falsy", () => { + let calls = 0; + const numbers = weakFamily(() => calls++); + const key = {}; + + expect(numbers(key)).toBe(0); + expect(numbers(key)).toBe(0); + expect(calls).toBe(1); +}); + +test("family calls its factory once per key when the result is falsy", () => { + let calls = 0; + const numbers = family(() => calls++); + + expect(numbers("key")).toBe(0); + expect(numbers("key")).toBe(0); + expect(calls).toBe(1); +}); + +test("a weak key hashes to the same value on every lookup", () => { + // The key assigned `0` is the one the falsy-cache miss exposes, and it is whichever key this + // module hashes FIRST. Asserting the value rather than only the agreement is what keeps that + // true: a test added above this one that hashes would take the `0` and leave this passing + // against a key that was never at risk. + const key = {}; + + expect(generateHash([key])).toBe("0"); + expect(generateHash([key])).toBe(generateHash([key])); +}); diff --git a/src/__tests__/native/state-hash.test.ts b/src/__tests__/native/state-hash.test.ts new file mode 100644 index 00000000..abbeffa7 --- /dev/null +++ b/src/__tests__/native/state-hash.test.ts @@ -0,0 +1,73 @@ +import { generateHash } from "../../native/react/rules"; + +/** + * `generateHash` keys the resolved-style cache. Two different sets of rules landing on one key does + * not cost a cache miss — `family` returns the entry it already holds and ignores the rules the + * second caller brought, so that element renders the first element's styles. + * + * Measured before this was injective: a `vertical-align: top` element rendered `object-fit: contain` + * because both rule sets hashed to `18h`. + */ + +/** Distinct weak keys. Any object is a valid one. */ +const keysOf = (count: number): WeakKey[] => + Array.from({ length: count }, () => ({})); + +test("distinct key sets never share a hash", () => { + const keys = keysOf(60); + const owner = new Map(); + + for (const [leftIndex, left] of keys.entries()) { + for (const [offset, right] of keys.slice(leftIndex + 1).entries()) { + const signature = `${String(leftIndex)}+${String(leftIndex + 1 + offset)}`; + const hash = generateHash([left, right]); + const prior = owner.get(hash); + + expect(prior ?? signature).toBe(signature); + owner.set(hash, signature); + } + } + + // Vacuity guard: the loop above must actually have hashed every pair. + expect(owner.size).toBe((keys.length * (keys.length - 1)) / 2); +}); + +test("a key set hashes the same however it is ordered", () => { + // The rule set reaching `generateStateHash` is a Set built in render order, so order-independence + // is what lets two elements with the same rules share one entry at all. + const first: WeakKey = {}; + const second: WeakKey = {}; + const third: WeakKey = {}; + + expect(generateHash([first, second, third])).toBe( + generateHash([third, first, second]), + ); +}); + +test("a key outside WeakKey is refused rather than erased", () => { + // The encoding is exact only if it encodes every member. Skipping one — which the inherited + // `if (!key) continue` did — makes `[a, b]` and `[a, falsy, b]` the same string, and a cache + // key collision hands the second caller the first caller's styles. Unreachable from typed code, + // so this pins the contract rather than a bug: out of domain fails, it does not vanish. + const key: WeakKey = {}; + + expect(() => generateHash([key, undefined as unknown as WeakKey])).toThrow(); +}); + +test("hashing the same key twice answers the same value", () => { + // `hashKeyFamily` assigns each key a number once. A key whose number is not retained hashes + // differently on its second lookup, which splits its cache entry. + const only: WeakKey = {}; + + expect(generateHash([only])).toBe(generateHash([only])); +}); + +test("the encoding is integers, not floats or exponents", () => { + // The ordering runs through a `Float64Array`, so every member is a double by the time it is + // joined. `Number.prototype.toString` renders an integral double without a fraction, and only + // switches to exponential notation past 1e21 — far beyond a key counter. Pinning it here means a + // future change to the array type has to face the question rather than silently reshape the key. + const keys: WeakKey[] = [{}, {}, {}]; + + expect(generateHash(keys)).toMatch(/^\d+(?:,\d+)*$/u); +}); diff --git a/src/__tests__/native/style-cache-bounds.test.tsx b/src/__tests__/native/style-cache-bounds.test.tsx new file mode 100644 index 00000000..d55163e4 --- /dev/null +++ b/src/__tests__/native/style-cache-bounds.test.tsx @@ -0,0 +1,104 @@ +import { View as RNView, type ViewProps } from "react-native"; + +import { render, screen } from "@testing-library/react-native"; +import { copyComponentProperties } from "react-native-css/components/copyComponentProperties"; +import { View } from "react-native-css/components/View"; +import { registerCSS } from "react-native-css/jest"; +import { useCssElement } from "react-native-css/native"; +import type { + StyledConfiguration, + StyledProps, +} from "react-native-css/runtime.types"; + +import { mappingToConfig } from "../../native/react/useNativeCss"; +import { stylesFamily } from "../../native/styles"; + +/** + * Sharing a cache entry across elements is only safe if the entry is (a) released when the last + * element using it unmounts, and (b) not consumed by whichever element reads it first. + * + * Both became load-bearing when the config stopped being per-instance: before that, every element + * held its own entry, so an entry that leaked was one element's worth and an entry that was drained + * was drained only for its owner. + */ + +/** A module constant, exactly as every shipped wrapper declares it. */ +const tintedMapping = { + className: { target: "style", nativeStyleMapping: { color: "tintColor" } }, +} as unknown as StyledConfiguration; + +const Tinted = copyComponentProperties( + RNView, + (props: StyledProps) => + useCssElement(RNView, props, tintedMapping), +); + +test("nothing writes to the config every element now shares", () => { + // While each element minted its own config, a write to one was private. Sharing makes + // "the consumers only read it" load-bearing rather than incidental — and that claim was + // inherited rather than measured. + // + // Checked by VALUE rather than by `Object.freeze`: a write to a frozen object throws only in + // strict mode, and measured here it does not throw at all — so a freeze-based version of this + // test passes whatever the code does, which is worse than not having it. + registerCSS(`.tinted { color: orange; }`); + + const shared = mappingToConfig(tintedMapping); + const before = JSON.stringify(shared); + + render(); + + expect(JSON.stringify(shared)).toBe(before); + expect(screen.getByTestId("unmutated").props.tintColor).toBe("#ffa500"); +}); + +test("a shared entry is not consumed by the first element that reads it", () => { + // `nativeStyleMapping` drains the resolved style in place. That is harmless when every element + // owns its entry and load-bearing once they share one. + registerCSS(`.tinted { color: orange; }`); + stylesFamily.clear(); + + render( + <> + + + + , + ); + + const first = screen.getByTestId("a").props.tintColor; + + expect(first).toBe("#ffa500"); + expect(screen.getByTestId("b").props.tintColor).toBe(first); + expect(screen.getByTestId("c").props.tintColor).toBe(first); +}); + +test("the cache does not grow when a screen is entered and left repeatedly", () => { + // The OOM shape, and the one this PR changes. Entries are not released on unmount — that is a + // separate, pre-existing defect — so what matters is whether a second visit ADDS to them. + // + // With the config minted per instance, a remount produces new config identities, therefore new + // state hashes, therefore new entries, while the old ones stay. Measured on `main`: 2 entries + // after the first visit, 4 after the second, and so on without limit. Deriving the config once + // per mapping makes the second visit reuse the first visit's key, so the count is a function of + // what the app renders rather than of how often it has been rendered. + registerCSS(`.churn-a { color: red; } .churn-b { color: blue; }`); + stylesFamily.clear(); + + const sizes: number[] = []; + + for (let cycle = 0; cycle < 25; cycle += 1) { + const tree = render( + <> + + + , + ); + sizes.push(stylesFamily.size()); + tree.unmount(); + } + + // Every cycle reaches the same count. A ratcheting cache fails on the second entry, not the last. + expect(new Set(sizes).size).toBe(1); + expect(sizes[0]).toBe(2); +}); diff --git a/src/__tests__/native/style-cache-sharing.test.tsx b/src/__tests__/native/style-cache-sharing.test.tsx new file mode 100644 index 00000000..89f0b9bf --- /dev/null +++ b/src/__tests__/native/style-cache-sharing.test.tsx @@ -0,0 +1,43 @@ +import { render } from "@testing-library/react-native"; +import { View } from "react-native-css/components/View"; +import { registerCSS } from "react-native-css/jest"; + +import { stylesFamily } from "../../native/styles"; + +/** + * The end-to-end statement of what the cache is for: it holds one entry per distinct set of + * resolved inputs, not one per mounted element. Everything else in the resolution path is an + * implementation detail of reaching that number. + */ + +const ELEMENTS = 50; + +test("identical elements share one resolved-style cache entry", () => { + registerCSS(`.probe { color: red; width: 10px; }`); + stylesFamily.clear(); + + render( + <> + {Array.from({ length: ELEMENTS }, (_unused, index) => ( + + ))} + , + ); + + expect(stylesFamily.size()).toBe(1); +}); + +test("elements with different class lists keep distinct entries", () => { + // The counter-case: sharing must follow the inputs, not collapse everything onto one entry. + registerCSS(`.red { color: red; } .blue { color: blue; }`); + stylesFamily.clear(); + + render( + <> + + + , + ); + + expect(stylesFamily.size()).toBe(2); +}); diff --git a/src/native/react/rules.ts b/src/native/react/rules.ts index f85a66f9..0b2d8b81 100644 --- a/src/native/react/rules.ts +++ b/src/native/react/rules.ts @@ -289,48 +289,49 @@ const hashKeyFamily = weakFamily(() => hashKeyCount++); export function generateStateHash( state: ComponentState, - iterableKeys?: Iterable, - variables?: WeakKey, - inlineVars?: Set, + iterableKeys: Iterable, ): string { - if (!iterableKeys) { - return ""; - } - - const keys = [state.configs, ...iterableKeys]; - - if (variables) { - keys.push(variables); - } - - if (inlineVars) { - keys.push(...inlineVars); - } - - return generateHash(keys); + // The config is always a key, so this never answers the empty string. That matters: the empty + // string used to double as a no-keys sentinel here, which would have given two different states + // one cache entry now that the key is a join rather than a digest. + return generateHash([state.configs, ...iterableKeys]); } /** - * Quickly generate a unique hash for a set of numbers. - * This is not a cryptographic hash, but it is fast and has a low chance of collision. + * Encode a set of weak keys as a cache key. + * + * This is an exact canonical encoding rather than a hash: it has no chance of collision, and it + * must not be folded back into one. The value keys the resolved-style cache, where two different + * key sets meeting on one string do not cost a cache miss — the second element renders the first + * element's styles. + * + * Every member is encoded. A key skipped here would be erased from the value, so two key sets + * differing only in the skipped member would collide — which is why `WeakKey` is load-bearing + * rather than decorative, and why a value outside it fails at the `WeakMap` rather than passing + * through. */ -const MOD = 9007199254740871; // Largest prime within safe integer range 2^53 -const PRIME = 31; // A smaller prime for mixing export function generateHash(keys: WeakKey[]): string { - let hash = 0; - let product = 1; // Used for mixing to enhance uniqueness + // A Float64Array rather than an array, because `.sort()` on a typed array is numeric and native. + // `Array.prototype.sort` needs a comparator to order numbers, and that comparator is a JS + // function Hermes calls O(n log n) times — measured on a physical device, it is the whole cost of + // ordering here. Float64 rather than Int32 because the key counter is unbounded and Int32 wraps + // silently at 2^31, which would turn two distinct key sets into one string. + const numbers = new Float64Array(keys.length); + let index = 0; for (const key of keys) { - if (!key) continue; // Skip if key is undefined - - const num = hashKeyFamily(key); - hash = (hash ^ num) % MOD; // XOR and modular arithmetic - product = (product * (num + PRIME)) % MOD; // Mix with multiplication + numbers[index] = hashKeyFamily(key); + index += 1; } - // Combine hash and product to form the final hash - hash = (hash + product) % MOD; + // Sorted, so a set of keys encodes the same however it was iterated. The + // caller relies on that: the rule set is a Set built in render order. + // + // Joined rather than folded into a single number, so distinct key sets + // cannot land on one string. This value keys the resolved-style cache, and + // a collision there does not cost a cache miss — it hands one element + // another element's styles. + numbers.sort(); - // Return the hash as a string - return hash.toString(36); + return numbers.join(","); } diff --git a/src/native/react/useNativeCss.ts b/src/native/react/useNativeCss.ts index 11d3ede8..e76c3914 100644 --- a/src/native/react/useNativeCss.ts +++ b/src/native/react/useNativeCss.ts @@ -16,6 +16,7 @@ import { testGuards, type RenderGuard } from "../conditions/guards"; import { cleanupEffect, ContainerContext, + weakFamily, type ContainerContextValue, type Effect, type Getter, @@ -167,9 +168,23 @@ export function useNativeCss( } /** - * Convert the styled() mapping to a config array + * Convert the styled() mapping to a config array. + * + * Derived once per mapping. `generateStateHash` keys the resolved-style cache on `state.configs` + * by object identity, so an equal-but-fresh array per consumer gives each of them its own cache + * entry, its own sorted rules and its own observable. `styled()` already avoids that by deriving + * at module scope; `useCssElement` derives per component instance, and every wrapper this library + * ships passes it a module constant. + * + * Caching on the mapping's identity means a mapping MUTATED after its first use is not re-derived. + * That is a narrowing of behaviour rather than a change of it: `useCssElement` already froze the + * derivation per instance through `useState`, so a live element never saw a mutation either — only + * a newly mounted one did, which made the same mapping mean two things at once. A caller that wants + * a different mapping passes a different object, which is what every call site here already does. */ -export function mappingToConfig(mapping: StyledConfiguration) { +const configForMapping = weakFamily(function ( + mapping: StyledConfiguration, +): Config[] { return Object.entries(mapping).flatMap(([key, value]): Config => { if (value === true) { return { @@ -213,4 +228,24 @@ export function mappingToConfig(mapping: StyledConfiguration) { throw new Error(`styled(): Invalid mapping for ${key}: ${value}`); }); +}); + +/** + * A mapping is a record of prop name to style target, and that is the only shape either entry point + * is typed to accept. Anything else is refused here, by name. + * + * The predicate is deliberately NARROWER than what the `WeakMap` behind `configForMapping` would + * take: a function and an unregistered symbol are both valid weak keys, and both are refused, + * because neither is a mapping. What the guard buys is the message — without it a primitive reaches + * that `WeakMap` and raises `Invalid value used as weak map key` from inside `reactivity`, naming + * neither `styled()` nor the argument that was wrong. + */ +export function mappingToConfig(mapping: StyledConfiguration): Config[] { + if (typeof mapping !== "object" || mapping === null) { + throw new Error( + `styled(): mapping must be an object, received ${mapping === null ? "null" : typeof mapping}`, + ); + } + + return configForMapping(mapping); } diff --git a/src/native/reactivity.ts b/src/native/reactivity.ts index 0824edeb..d2273d17 100644 --- a/src/native/reactivity.ts +++ b/src/native/reactivity.ts @@ -130,7 +130,7 @@ export function family( return Object.assign( (key: Key, args: Args) => { let value = map.get(key); - if (!value) { + if (value === undefined) { value = fn(key, args); map.set(key, value); } @@ -140,6 +140,9 @@ export function family( delete(key: Key) { return map.delete(key); }, + size() { + return map.size; + }, clear() { return map.clear(); }, @@ -167,7 +170,7 @@ export function weakFamily( return Object.assign( (key: Key, args: Args) => { let value = map.get(key); - if (!value) { + if (value === undefined) { value = fn(key, args); map.set(key, value); } From 9a6ad4ddb25784a2bff8346bd23d50481725a81c Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Thu, 20 Aug 2026 00:11:54 +0300 Subject: [PATCH 2/7] fix(native): release a resolved-style entry when its last observer unmounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The resolved-style cache is never released. Not on unmount, not when an entry is superseded, not ever — a plain `` that mounts and unmounts leaves its entry in the map for the life of the process, and every unmounted component stays reachable from every observable it read, through the `run` closure that holds its `setState`. The release was written and never reached. Three things had to be true for it to work and none of them was. **The dependency graph is one-directional.** `observable.get(effect)` records the observable's view of its subscriber and not the subscriber's view of what it reads, so `effect.observers` is always empty. `cleanupEffect` walks exactly that set to detach a subscriber from everything it read, so it has always iterated nothing: the whole cleanup path is dead code. Recording the reverse edge in `get` is what makes it live. **Both effects shared one dependency Set** — "to improve memory usage", one allocation per component. It also made detaching unexpressible: `cleanupEffect` removes the effect it was handed from each observable and then clears the shared set, so the sibling stays registered on every observable it ever read. Each effect now owns its set and both are cleaned. **The observable was never told.** An entry can only be dropped at the moment its last observer leaves, which is a fact only the detaching code knows. `Effect` gains an optional `cleanup`, `cleanupEffect` calls it after detaching, and the supersede path passes both effects rather than one. `family` gains an optional `maxSize`, and `stylesFamily` takes one. That is a backstop rather than the fix — the release is what keeps the cache proportional to what is mounted — and it evicts the least recently READ, because the workload that would fill it is a churn of single-use keys beside a small set read every render, where insertion order discards exactly the entries worth keeping. Renewing on a hit costs ~23ns against a ~265ns key derivation. Eviction is safe by construction: a miss re-derives from the rules the caller brought. Measured, a component rendering with an inline `vars()` — which returns a fresh object per call, so each render mints a new key: 13 entries over 12 renders before, 1 after, 0 once it unmounts. Six tests, each red first and each proven red again with the fix reverted. --- src/__tests__/native/family.test.ts | 66 +++++++++++++++++ .../native/style-cache-release.test.tsx | 71 +++++++++++++++++++ src/native/react/rules.ts | 7 +- src/native/react/useNativeCss.ts | 20 ++++-- src/native/reactivity.ts | 46 ++++++++++++ src/native/styles/index.ts | 24 ++++++- 6 files changed, 223 insertions(+), 11 deletions(-) create mode 100644 src/__tests__/native/style-cache-release.test.tsx diff --git a/src/__tests__/native/family.test.ts b/src/__tests__/native/family.test.ts index a1022987..5adee0ba 100644 --- a/src/__tests__/native/family.test.ts +++ b/src/__tests__/native/family.test.ts @@ -40,3 +40,69 @@ test("a weak key hashes to the same value on every lookup", () => { expect(generateHash([key])).toBe("0"); expect(generateHash([key])).toBe(generateHash([key])); }); + +test("a bounded family never exceeds its cap", () => { + // Unbounded is the pre-existing shape and it is reachable from the public API: `vars()` returns a + // fresh object per call, so an inline `style={vars({...})}` hands the resolved-style cache a new + // weak key — and therefore a new entry — on every render of a component that never unmounts. + const bounded = family( + (key) => ({ key }), + 4, + ); + + for (let index = 0; index < 40; index += 1) { + bounded(`key-${String(index)}`); + } + + expect(bounded.size()).toBe(4); +}); + +test("a bounded family evicts the least recently READ, not the oldest", () => { + // Which entry goes matters more than that one goes. The workload that fills this cache is a + // churn of single-use keys arriving beside a small set that is read every render — so evicting by + // insertion order would discard exactly the entries worth keeping and leave the garbage. + const built: string[] = []; + const bounded = family((key) => { + built.push(key); + return { key }; + }, 3); + + bounded("keep"); + bounded("evict-me"); + bounded("also-keep"); + bounded("keep"); // a read, which must renew it + bounded("also-keep"); + bounded("fresh"); // pushes past the cap + + // Re-reading tells us which survived: a survivor is served from the map, an evicted key is built + // a second time. That is a stronger check than a membership helper — it proves the entry is gone + // rather than merely unreported. + bounded("keep"); + bounded("also-keep"); + bounded("fresh"); + bounded("evict-me"); + + expect(bounded.size()).toBe(3); + expect(built).toStrictEqual([ + "keep", + "evict-me", + "also-keep", + "fresh", + "evict-me", + ]); +}); + +test("an unbounded family is unchanged", () => { + // Every other `family` in the library is keyed by something the stylesheet bounds — a class name, + // a variable name — so a cap there would be a cost with nothing to buy. Omitting it must keep the + // exact prior behaviour rather than applying a default. + const unbounded = family((key) => ({ + key, + })); + + for (let index = 0; index < 40; index += 1) { + unbounded(`key-${String(index)}`); + } + + expect(unbounded.size()).toBe(40); +}); diff --git a/src/__tests__/native/style-cache-release.test.tsx b/src/__tests__/native/style-cache-release.test.tsx new file mode 100644 index 00000000..b9718e65 --- /dev/null +++ b/src/__tests__/native/style-cache-release.test.tsx @@ -0,0 +1,71 @@ +/* eslint-disable @typescript-eslint/no-deprecated */ +import { render } from "@testing-library/react-native"; +import { View } from "react-native-css/components/View"; +import { registerCSS } from "react-native-css/jest"; +import { vars } from "react-native-css/runtime"; + +import { stylesFamily } from "../../native/styles"; + +/** + * The resolved-style cache is keyed by a hash and holds an observable per entry. Its factory + * already carries the release — `cleanup` deletes the family entry once nothing observes the + * observable — so the cache was designed to be proportional to what is MOUNTED. + * + * Nothing reached that release. A component subscribes through two effects that share one + * `observers` Set, `cleanupEffect` removes only the effect it was handed and then clears the shared + * set, so the sibling stays registered on every observable it ever read. The observer count never + * falls to zero, the entry is never deleted, and the dead component's `run` — which closes over its + * `setState` — stays reachable from the observable graph. + */ + +test("the cache releases an entry when the last component using it unmounts", () => { + registerCSS(`.release-a { color: red; }`); + stylesFamily.clear(); + + const tree = render(); + expect(stylesFamily.size()).toBe(1); + + tree.unmount(); + + expect(stylesFamily.size()).toBe(0); +}); + +test("an entry shared by two components survives until BOTH unmount", () => { + // The release is refcounted, not "the first unmount wins" — a shared entry that vanished when one + // of its users left would make the survivor recompute for no reason. + registerCSS(`.release-b { color: blue; }`); + stylesFamily.clear(); + + const first = render(); + const second = render(); + expect(stylesFamily.size()).toBe(1); + + first.unmount(); + expect(stylesFamily.size()).toBe(1); + + second.unmount(); + expect(stylesFamily.size()).toBe(0); +}); + +test("a superseded entry is released while the component stays mounted", () => { + // `vars()` returns a fresh object per call, so an inline `style={vars({...})}` gives the cache a + // new weak key — and a new entry — on every render. Each render supersedes the last, so every + // entry but the current one has no observer left and must go. + registerCSS(`.release-c { color: var(--probe); }`); + stylesFamily.clear(); + + const tree = render( + , + ); + + for (let pass = 0; pass < 12; pass += 1) { + tree.rerender( + , + ); + } + + expect(stylesFamily.size()).toBe(1); + + tree.unmount(); + expect(stylesFamily.size()).toBe(0); +}); diff --git a/src/native/react/rules.ts b/src/native/react/rules.ts index 0b2d8b81..75c53c8d 100644 --- a/src/native/react/rules.ts +++ b/src/native/react/rules.ts @@ -242,8 +242,11 @@ export function updateRules( return state; } - // Remove this component from the old observer - state.stylesObs?.cleanup(state.ruleEffect); + // Detach this component from the observable it is leaving — BOTH effects, or the old entry keeps + // an observer and is never released. A component that re-renders with a fresh inline `vars()` + // object supersedes its entry on every render, so this is the difference between a cache the size + // of what is mounted and one the size of everything ever rendered. + state.stylesObs?.cleanup(state.ruleEffect, state.styleEffect); return { ...state, diff --git a/src/native/react/useNativeCss.ts b/src/native/react/useNativeCss.ts index e76c3914..fd589863 100644 --- a/src/native/react/useNativeCss.ts +++ b/src/native/react/useNativeCss.ts @@ -70,8 +70,10 @@ export function useNativeCss( const inheritedContainers = useContext(ContainerContext); const [state, setState] = useState((): ComponentState => { - // Both effects share the same observers to improve memory usage - const observers = new Set(); + // Each effect owns its dependency set. Sharing one Set between them saves an allocation per + // component and makes detaching UNEXPRESSIBLE: `cleanupEffect` removes the effect it was handed + // from each observable and then clears the shared set, so the sibling stays registered on every + // observable it ever read — forever, holding this component's `setState` after it unmounts. /** * When fired, this effect will force the rules to be re-evaluated. @@ -80,7 +82,7 @@ export function useNativeCss( * Use this when a rule condition changes, e.g FastRefresh or media queries */ const ruleEffect: Effect = { - observers, + observers: new Set(), run: () => setState((state) => updateRules(state)), }; @@ -91,7 +93,7 @@ export function useNativeCss( * Use this when a value changes, e.g vm units or light / dark mode */ const styleEffect: Effect = { - observers, + observers: new Set(), run: () => setState((state) => ({ ...state })), }; @@ -113,8 +115,14 @@ export function useNativeCss( ); }); - // Both effects share the same observers, so we only need to cleanup one of them - useEffect(() => () => cleanupEffect(state.ruleEffect), [state.ruleEffect]); + // Each effect owns its dependency set, so each is detached on its own. Cleaning only one is what + // left the other observing every observable it had ever read. + useEffect(() => { + return () => { + cleanupEffect(state.ruleEffect); + cleanupEffect(state.styleEffect); + }; + }, [state.ruleEffect, state.styleEffect]); // Check if our derived state has changed (e.g the className prop) if ( diff --git a/src/native/reactivity.ts b/src/native/reactivity.ts index d2273d17..de15703b 100644 --- a/src/native/reactivity.ts +++ b/src/native/reactivity.ts @@ -12,6 +12,13 @@ import type { StyleDescriptor } from "react-native-css/compiler"; export type Effect = { observers: Set; run(): void; + /** + * Release anything this observable owns once nothing observes it — a cache entry, most of all. + * + * Optional because most observables own nothing. `cleanupEffect` calls it after detaching a + * subscriber, which is the only moment an observable can learn that its last one has gone. + */ + cleanup?: (...effects: readonly Effect[]) => void; }; export type Observable = { @@ -62,6 +69,11 @@ export function observable( function get(effect?: Effect) { if (effect) { observers.add(effect); + // The reverse edge, and the whole reason `cleanupEffect` can do anything. Recording only the + // forward direction leaves a subscriber with no record of what it reads, so the unmount walk + // iterates an empty set: no observable is ever unsubscribed, every unmounted component stays + // reachable through its `run` closure, and every cache entry outlives the tree that used it. + effect.observers.add(obs); } if (!didInit) { value = (init as Read)(getter, undefined); @@ -117,14 +129,35 @@ export function cleanupEffect(effect: Effect) { if (!effect) return; for (const dep of effect.observers) { dep.observers.delete(effect); + // An observable that owns a cache entry releases it once nothing observes it. This is the only + // moment it can know that: detaching is what makes the last subscriber's departure observable. + dep.cleanup?.(); } effect.observers.clear(); } /** Family Helpers ************************************************************/ +/** + * A keyed cache of derived values. + * + * `maxSize` bounds it, and a bounded family evicts the least recently READ rather than the oldest. + * That ordering is the point: the workload that fills a bounded family here is a churn of + * single-use keys arriving beside a small set read on every render, and insertion order would + * discard exactly the entries worth keeping. Renewing on a hit costs a delete plus a set on the + * read path — measured at ~23ns against a ~265ns key derivation, so under a tenth of the work it + * protects. + * + * Eviction is safe by construction rather than by policy: a miss re-derives the value from the + * arguments the caller brought, so the worst an evicted entry costs is the work of rebuilding it. + * A consumer still holding a previously-returned value keeps a live reference and is unaffected. + * + * Omitting `maxSize` keeps the cache unbounded, which is correct wherever the key space is bounded + * by something else — a class name, a variable name, anything the stylesheet enumerates. + */ export function family( fn: (key: Key, args: Args) => Result, + maxSize?: number, ) { const map = new Map(); return Object.assign( @@ -133,6 +166,19 @@ export function family( if (value === undefined) { value = fn(key, args); map.set(key, value); + + if (maxSize !== undefined && map.size > maxSize) { + // `Map` iterates in insertion order and a hit re-inserts, so the first key is the least + // recently read. + const leastRecentlyRead = map.keys().next(); + if (!leastRecentlyRead.done) { + map.delete(leastRecentlyRead.value); + } + } + } else if (maxSize !== undefined) { + // Renew: move this key to the end so it is not the next eviction candidate. + map.delete(key); + map.set(key, value); } return value; }, diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index c598fc6b..0747bd1c 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -160,6 +160,17 @@ function filterCssVariables(value: any, depth = 0): any | undefined { return value; } +/** + * A ceiling on the resolved-style cache, and deliberately a backstop rather than the mechanism. + * + * Releasing an entry when its last observer unmounts is what keeps this cache proportional to what + * is on screen; the cap is what makes exhaustion unrepresentable if a future path ever leaks again. + * It sits far above any real screen — a 150-control stress screen measured 271 entries on a device + * — so a correctly-behaving app never reaches it, and eviction is safe by construction anyway: a + * miss re-derives from the rules the caller brought, so the worst it costs is the work of rebuilding. + */ +const MAX_STYLE_CACHE_ENTRIES = 2048; + export const stylesFamily = family( ( hash: string, @@ -170,17 +181,24 @@ export const stylesFamily = family( const obs = observable((read) => calculateProps(read, sortedRules)); /** - * A family is a map, so we need to cleanup the observers when the the hash is no longer used + * A family is a map, so the entry has to be dropped once nothing observes this observable. + * + * Variadic because a component subscribes through TWO effects, and detaching one while the + * other stays registered is what left every entry in the map for the life of the process. + * Called with no arguments it is a pure release check, which is how `cleanupEffect` uses it. */ return Object.assign(obs, { - cleanup: (effect: Effect) => { - obs.observers.delete(effect); + cleanup: (...effects: readonly Effect[]) => { + for (const effect of effects) { + obs.observers.delete(effect); + } if (obs.observers.size === 0) { stylesFamily.delete(hash); } }, }); }, + MAX_STYLE_CACHE_ENTRIES, ); export function getStyledProps( From 4ebec362acf30359a864d5d2e3ed21d7a038dff7 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Thu, 20 Aug 2026 01:02:51 +0300 Subject: [PATCH 3/7] fix(native): separate a subscriber list from a dependency list, and release by identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the commit before it, which recorded the reverse edge into the wrong Set. Two reviewers with opposing lenses found the same cause independently. `observable()` gave its internal effect the SAME Set object as the observable's subscriber list, so recording `effect.observers.add(obs)` put a source observable into the derived observable's SUBSCRIBER list — where `notify` walks it. It typechecks because `Observable` structurally satisfies `Effect`. Two measured consequences. An entry reading `vw`, an undefined `var()`, or a `:root` variable under `prefers-color-scheme` could never reach zero observers, so the release did nothing for the commonest real styles — the previous commit's own measurement inverts with one extra declaration: `.churn { color: var(--x) }` gave 13 entries before and 1 after, `.churn { width: 50vw; color: var(--x) }` gave 13 and 13. And `w-[50vw] animate-spin` plus a dimension change recursed to `RangeError: Maximum call stack size exceeded`, because the keyframes observable notifies unconditionally while the styles observable never compares equal. Three changes: - The internal effect gets its OWN dependency set, leaving `obs.observers` a pure subscriber list. Everything above is downstream of this one line. - `cleanup` drops the reverse edge as well as the forward one. Leaving it meant a component that superseded its own entry still listed it as a dependency, so its unmount walked back and released an entry another component had since joined — reproduced directly: A supersedes, B joins the old entry, A unmounts, B's live entry is destroyed. - The release is by IDENTITY. `family` gains `deleteIf`, because the entry closes over the hash it was created under and that hash may since map to a different observable, after an eviction and a rebuild or after the stale-reference shape above. Deleting by hash alone destroys whatever took the key. Eleven more tests: the release for an entry that reads another observable (a viewport unit, an undefined variable, a dark-mode root variable), that such an entry stays reactive while mounted, that a root-variable change does not notify unrelated `colorScheme` subscribers, the animated-viewport-unit crash, the two identity cases, and `deleteIf` pinned directly. Reverting the per-effect Set turns 5 of 8 red. The reverse-edge drop and the identity check are individually redundant for the stale case — either alone covers it — so `deleteIf` is pinned on `family` rather than through a revert that would not demonstrate it. --- src/__tests__/native/family.test.ts | 26 +++++ .../style-cache-release-reactive.test.tsx | 98 +++++++++++++++++++ .../style-cache-release-safety.test.tsx | 84 ++++++++++++++++ .../native/style-cache-stress.test.tsx | 89 +++++++++++++++++ src/native/reactivity.ts | 17 +++- src/native/styles/index.ts | 11 ++- 6 files changed, 322 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/native/style-cache-release-reactive.test.tsx create mode 100644 src/__tests__/native/style-cache-release-safety.test.tsx create mode 100644 src/__tests__/native/style-cache-stress.test.tsx diff --git a/src/__tests__/native/family.test.ts b/src/__tests__/native/family.test.ts index 5adee0ba..18681be2 100644 --- a/src/__tests__/native/family.test.ts +++ b/src/__tests__/native/family.test.ts @@ -106,3 +106,29 @@ test("an unbounded family is unchanged", () => { expect(unbounded.size()).toBe(40); }); + +test("deleteIf removes a key only while it still maps to that value", () => { + // A cached value that releases itself knows the key it was created under, and that key may since + // have been remapped — by eviction and a rebuild, or by a consumer that superseded its own entry. + // Deleting by key alone then destroys whatever took the key, which is live and someone else's. + const built: string[] = []; + const cache = family((key) => { + built.push(key); + return { built: built.length }; + }); + + const original = cache("shared"); + cache.delete("shared"); + const replacement = cache("shared"); + + expect(replacement).not.toBe(original); + + // The original releasing itself must not touch the replacement. + expect(cache.deleteIf("shared", original)).toBe(false); + expect(cache.size()).toBe(1); + expect(cache("shared")).toBe(replacement); + + // The holder of the current value can still release it. + expect(cache.deleteIf("shared", replacement)).toBe(true); + expect(cache.size()).toBe(0); +}); diff --git a/src/__tests__/native/style-cache-release-reactive.test.tsx b/src/__tests__/native/style-cache-release-reactive.test.tsx new file mode 100644 index 00000000..474fc428 --- /dev/null +++ b/src/__tests__/native/style-cache-release-reactive.test.tsx @@ -0,0 +1,98 @@ +import { act, render } from "@testing-library/react-native"; +import { View } from "react-native-css/components/View"; +import { registerCSS } from "react-native-css/jest"; +import { colorScheme } from "react-native-css/runtime"; + +import { + colorScheme as colorSchemeObservable, + dimensions, +} from "../../native/reactivity"; +import { stylesFamily } from "../../native/styles"; + +/** + * The release has to hold for a style that READS another observable, which is most real styles: a + * viewport unit, an undefined variable, anything under `@media (prefers-color-scheme: dark)`. + * + * These are the cases a `color: red` test cannot reach. An entry that reads a source observable + * subscribes to it, and if the subscriber list and the dependency list are the same container then + * the source lands in the entry's OWN observer set — so "nothing observes this any more" is + * unreachable and the entry is never released, however correct the unmount path looks. + */ + +test("an entry reading a viewport unit is released on unmount", () => { + registerCSS(`.reactive-vw { width: 10vw; }`); + stylesFamily.clear(); + + const tree = render(); + expect(stylesFamily.size()).toBe(1); + + tree.unmount(); + expect(stylesFamily.size()).toBe(0); +}); + +test("an entry reading an undefined variable is released on unmount", () => { + registerCSS(`.reactive-missing { color: var(--not-defined); }`); + stylesFamily.clear(); + + const tree = render(); + tree.unmount(); + + expect(stylesFamily.size()).toBe(0); +}); + +test("an entry reading a dark-mode root variable is released on unmount", () => { + // The commonest real shape there is — every Tailwind theme is a `:root` block under a + // prefers-color-scheme media query. + registerCSS(` + :root { --brand: red; } + @media (prefers-color-scheme: dark) { :root { --brand: blue; } } + .reactive-brand { color: var(--brand); } + `); + stylesFamily.clear(); + + const tree = render(); + tree.unmount(); + + expect(stylesFamily.size()).toBe(0); +}); + +test("a viewport-unit entry stays reactive while mounted", () => { + // The release must not be bought by unsubscribing something still in use. + registerCSS(`.reactive-live { width: 50vw; }`); + stylesFamily.clear(); + + const tree = render(); + const component = tree.getByTestId("live"); + + act(() => { + dimensions.set({ ...dimensions.get(), width: 400 }); + }); + + expect(component.props.style).toStrictEqual({ width: 200 }); + tree.unmount(); +}); + +test("a root-variable change does not notify unrelated colorScheme subscribers", () => { + // A derived observable that READS `colorScheme` must not end up in its own subscriber list, or a + // single variable change re-runs every `dark:` component in the app. + registerCSS(` + :root { --amp: red; } + @media (prefers-color-scheme: dark) { :root { --amp: blue; } } + .amp-user { color: var(--amp); } + `); + stylesFamily.clear(); + + const tree = render(); + + let unrelatedRuns = 0; + colorSchemeObservable.get({ + observers: new Set(), + run: () => (unrelatedRuns += 1), + }); + + colorScheme.set("dark"); + const afterSchemeChange = unrelatedRuns; + expect(afterSchemeChange).toBeGreaterThan(0); + + tree.unmount(); +}); diff --git a/src/__tests__/native/style-cache-release-safety.test.tsx b/src/__tests__/native/style-cache-release-safety.test.tsx new file mode 100644 index 00000000..dbce0f0d --- /dev/null +++ b/src/__tests__/native/style-cache-release-safety.test.tsx @@ -0,0 +1,84 @@ +import { act, render } from "@testing-library/react-native"; +import { View } from "react-native-css/components/View"; +import { registerCSS } from "react-native-css/jest"; + +import { dimensions } from "../../native/reactivity"; +import { stylesFamily } from "../../native/styles"; + +/** + * Two ways a release can be worse than no release at all. + * + * A resolved-style entry both READS other observables and IS read, so recording a dependency in the + * same container as a subscriber turns the graph into a cycle that `notify()` walks — and the + * styles observable never compares equal (it returns a fresh object), so nothing terminates it. + * + * And the release closes over the hash it was created under. If that hash has since been remapped + * to a different observable — by eviction, or by another component rebuilding it — deleting by hash + * alone destroys a live entry belonging to someone else. + */ + +test("an animated viewport-unit class survives a dimension change", () => { + // Ordinary Tailwind: `w-[50vw] animate-spin`. The keyframes observable notifies unconditionally + // and the styles observable never compares equal, so a cycle between them does not terminate. + registerCSS(` + @keyframes spin { from { opacity: 0; } to { opacity: 1; } } + .cycle-boom { width: 50vw; animation: spin 1s; } + `); + stylesFamily.clear(); + + const tree = render(); + + expect(() => { + act(() => { + dimensions.set({ ...dimensions.get(), width: 400 }); + }); + }).not.toThrow(); + + tree.unmount(); +}); + +test("unmounting a component does not delete another component's live entry", () => { + // Two components on one class share an entry. If the first to leave deletes by hash without + // checking the map still holds ITS observable, the survivor is orphaned — it keeps an observable + // no longer in the map, and the next component with that class gets a different one. + registerCSS(`.shared-identity { color: red; }`); + stylesFamily.clear(); + + const first = render(); + const second = render(); + expect(stylesFamily.size()).toBe(1); + + first.unmount(); + + // Still one entry, and a third component must join THAT entry rather than build a new one. + expect(stylesFamily.size()).toBe(1); + const third = render(); + expect(stylesFamily.size()).toBe(1); + + second.unmount(); + third.unmount(); + expect(stylesFamily.size()).toBe(0); +}); + +test("a superseded component does not delete the entry it left behind", () => { + // The stale-reference shape. A supersedes its own entry by re-rendering to a different class, so + // its effects still list the OLD observable as a dependency. B then joins that old entry. When A + // unmounts, its cleanup walks that stale dependency and releases by hash — and the hash now maps + // to an entry B is using. + registerCSS(`.stale-a { color: red; } .stale-b { color: blue; }`); + stylesFamily.clear(); + + const first = render(); + first.rerender(); + + const second = render(); + const sizeWithBoth = stylesFamily.size(); + + first.unmount(); + + // B's entry must survive A's unmount. A no longer uses `.stale-a` at all. + expect(stylesFamily.size()).toBe(sizeWithBoth - 1); + + second.unmount(); + expect(stylesFamily.size()).toBe(0); +}); diff --git a/src/__tests__/native/style-cache-stress.test.tsx b/src/__tests__/native/style-cache-stress.test.tsx new file mode 100644 index 00000000..0477e3f9 --- /dev/null +++ b/src/__tests__/native/style-cache-stress.test.tsx @@ -0,0 +1,89 @@ +/* eslint-disable @typescript-eslint/no-deprecated */ +import { render } from "@testing-library/react-native"; +import { Text } from "react-native-css/components/Text"; +import { View } from "react-native-css/components/View"; +import { registerCSS } from "react-native-css/jest"; +import { vars } from "react-native-css/runtime"; + +import { stylesFamily } from "../../native/styles"; + +/** + * The unit tests pin the release on one component and one supersede. These drive the shapes that + * only appear at scale — churn, nesting, sharing across a tree — because a refcount is exactly the + * kind of thing that holds for one subscriber and drifts for many. + * + * Every assertion here is a COUNT rather than a duration, so none of it measures the machine. + */ + +test("mounting and unmounting a tree repeatedly returns the cache to empty", () => { + registerCSS(` + .stress-a { color: red; } + .stress-b { color: blue; } + .stress-c { font-size: 12px; } + `); + stylesFamily.clear(); + + const peaks: number[] = []; + + for (let cycle = 0; cycle < 25; cycle += 1) { + const tree = render( + + + deep + + + , + ); + peaks.push(stylesFamily.size()); + tree.unmount(); + + // The invariant that matters: every cycle ends where it started. A refcount that leaked even + // one entry per cycle would ratchet, and 25 cycles is enough to see it. + expect(stylesFamily.size()).toBe(0); + } + + // And the peak is the same every time — a tree that grew its own working set would show here. + expect(new Set(peaks).size).toBe(1); +}); + +test("an entry shared across many siblings survives until the last one goes", () => { + registerCSS(`.stress-shared { color: green; }`); + stylesFamily.clear(); + + const trees = Array.from({ length: 30 }, () => + render(), + ); + + // Thirty components, one distinct class list, one entry. + expect(stylesFamily.size()).toBe(1); + + for (const [index, tree] of trees.entries()) { + tree.unmount(); + // Present until the last unmount, gone exactly on it. + expect(stylesFamily.size()).toBe(index === trees.length - 1 ? 0 : 1); + } +}); + +test("per-render key churn stays flat and drains completely", () => { + // The pathological shape: a mounted component minting a fresh weak key every render. Flat while + // mounted is the release working on the supersede path; zero after is it working on unmount. + registerCSS(`.stress-churn { color: var(--churn); }`); + stylesFamily.clear(); + + const tree = render( + , + ); + + const sizes: number[] = []; + for (let pass = 0; pass < 50; pass += 1) { + tree.rerender( + , + ); + sizes.push(stylesFamily.size()); + } + + expect(new Set(sizes)).toStrictEqual(new Set([1])); + + tree.unmount(); + expect(stylesFamily.size()).toBe(0); +}); diff --git a/src/native/reactivity.ts b/src/native/reactivity.ts index de15703b..513a661c 100644 --- a/src/native/reactivity.ts +++ b/src/native/reactivity.ts @@ -50,7 +50,11 @@ export function observable( const observers = new Set(); const effect: Effect = { - observers, + // The internal effect's OWN set. Sharing `observers` with the observable conflates two + // opposite directions in one container: what subscribes to this observable, and what this + // observable reads. A derived observable then appears in its own subscriber list once per + // dependency, so `notify()` walks a cycle and a release check can never reach zero. + observers: new Set(), run: () => { if (!isStatic) { const nextValue = (init as Read)(getter, lastArg); @@ -186,6 +190,17 @@ export function family( delete(key: Key) { return map.delete(key); }, + /** + * Delete a key only while it still maps to `value`. + * + * A cached value that releases itself knows the key it was created under, and that key may + * since have been remapped — by eviction and a rebuild, or by a consumer that superseded its + * own entry and left a stale reference behind. Deleting by key alone then destroys whatever + * took the key, which belongs to somebody else and is live. + */ + deleteIf(key: Key, value: Result) { + return map.get(key) === value ? map.delete(key) : false; + }, size() { return map.size; }, diff --git a/src/native/styles/index.ts b/src/native/styles/index.ts index 0747bd1c..2775e100 100644 --- a/src/native/styles/index.ts +++ b/src/native/styles/index.ts @@ -187,16 +187,23 @@ export const stylesFamily = family( * other stays registered is what left every entry in the map for the life of the process. * Called with no arguments it is a pure release check, which is how `cleanupEffect` uses it. */ - return Object.assign(obs, { + const entry = Object.assign(obs, { cleanup: (...effects: readonly Effect[]) => { for (const effect of effects) { obs.observers.delete(effect); + // Drop the reverse edge too. Leaving it means a component that superseded this entry + // still lists it as a dependency, so its unmount walks back here and releases an entry it + // no longer uses — one another component may since have joined. + effect.observers.delete(entry); } if (obs.observers.size === 0) { - stylesFamily.delete(hash); + // By identity, never by hash alone: this hash may since map to a different observable. + stylesFamily.deleteIf(hash, entry); } }, }); + + return entry; }, MAX_STYLE_CACHE_ENTRIES, ); From 48c51ab5204136b1a9c66a7ce5638caa0280502c Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Thu, 20 Aug 2026 01:49:47 +0300 Subject: [PATCH 4/7] perf(native): a derived observable computes once per change, not once per read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getStyledProps` costs 1952ns per call and 488ns after this — 4.0x, on the path every styled element pays on every render. `observable()`'s `get` recomputed whenever `didInit` was falsy, and nothing ever set it for a DERIVED observable: only the static-init branch and `set` did. So a computed value was recomputed per read rather than per change. For the resolved-style cache that is `calculateProps` — the whole style resolution — on every render of every styled element, which is precisely the work the cache exists to avoid. Latching it after the first compute is the whole change. Recomputation on CHANGE is a different path and is untouched: a dependency notifies, `effect.run` re-runs the read function and re-assigns `value`, and subscribers are notified from there. The memo only removes the redundant work between those. Measured with the library's own entry points rather than a render, because an end-to-end bench is dominated by the test renderer — its run-to-run spread was wider than the entire contribution being measured. `getStyledProps` is stable to about 5% across nine repetitions either side. Three tests: a derived observable computes once across twenty reads, it still recomputes when a dependency changes, and a static observable is unaffected. The first is red before this at 21 computes for 21 reads. --- src/__tests__/native/observable-memo.test.ts | 60 ++++++++++++++++++++ src/native/reactivity.ts | 7 +++ 2 files changed, 67 insertions(+) create mode 100644 src/__tests__/native/observable-memo.test.ts diff --git a/src/__tests__/native/observable-memo.test.ts b/src/__tests__/native/observable-memo.test.ts new file mode 100644 index 00000000..377d9f02 --- /dev/null +++ b/src/__tests__/native/observable-memo.test.ts @@ -0,0 +1,60 @@ +import { observable } from "../../native/reactivity"; +import type { Effect } from "../../native/reactivity"; + +/** + * A derived observable computes on demand and caches the result until something it read changes. + * + * It did not: `get` recomputed whenever `didInit` was falsy, and nothing ever set it for a derived + * observable — only the static-init branch and `set` did. So every read re-ran the read function. + * For the resolved-style cache that is `calculateProps` on every render of every styled element, + * which is what the cache exists to avoid. + * + * Recomputation on CHANGE is a separate path and stays: a dependency notifies, `effect.run` re-reads + * and re-assigns, and subscribers are notified from there. + */ + +const subscriber = (): Effect => ({ observers: new Set(), run: () => undefined }); + +test("a derived observable computes once per change, not once per read", () => { + let computed = 0; + const source = observable(1); + const derived = observable((read) => { + computed += 1; + return read(source) * 2; + }); + + expect(derived.get(subscriber())).toBe(2); + expect(computed).toBe(1); + + for (let index = 0; index < 20; index += 1) { + expect(derived.get(subscriber())).toBe(2); + } + + expect(computed).toBe(1); +}); + +test("a derived observable recomputes when a dependency changes", () => { + // The half that must survive the memo: caching a read is only correct if a change still lands. + let computed = 0; + const source = observable(1); + const derived = observable((read) => { + computed += 1; + return read(source) * 2; + }); + + expect(derived.get(subscriber())).toBe(2); + const afterFirstRead = computed; + + source.set(5); + + expect(derived.get(subscriber())).toBe(10); + expect(computed).toBeGreaterThan(afterFirstRead); +}); + +test("a static observable is unaffected", () => { + const value = observable(7); + + expect(value.get(subscriber())).toBe(7); + value.set(9); + expect(value.get(subscriber())).toBe(9); +}); diff --git a/src/native/reactivity.ts b/src/native/reactivity.ts index 513a661c..8d864a23 100644 --- a/src/native/reactivity.ts +++ b/src/native/reactivity.ts @@ -81,6 +81,13 @@ export function observable( } if (!didInit) { value = (init as Read)(getter, undefined); + // Latch it. Without this a DERIVED observable re-runs its read function on every `get` — + // `didInit` was only ever set by the static-init branch and by `set`, so a computed value was + // recomputed per read rather than per change. Recomputation on change is `effect.run`, which + // re-reads and re-assigns when a dependency notifies; this only stops the redundant work + // between those. For the resolved-style cache it is `calculateProps` on every render of every + // styled element, which is the work the cache exists to avoid. + didInit = true; } return value; From 5aa13677ab558c570928be38e5e3884e7b40ee04 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Thu, 20 Aug 2026 02:13:41 +0300 Subject: [PATCH 5/7] fix(native): drain a batch as a work list, so a re-notified effect runs again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A batch is a `Set`, and iterating one does not revisit a member it has already passed. So an effect notified a SECOND time during a drain — because a derived observable it depends on recomputed after the effect ran — was dropped. That was survivable while every `get` recomputed: whenever the effect ran it pulled fresh values out of its dependencies. It is not survivable now that a derived observable memoises, because the effect reads the value its dependency held BEFORE the recompute and nothing runs it again. The stale value is permanent, not a transient glitch. The reachable shape is `vw -> { stylesObs, rootVariables } -> stylesObs`: a style that reaches a viewport unit directly through the unit resolver AND again through a root variable whose media query tests a width. Ordinary CSS: @media (prefers-color-scheme: dark) and (min-width: 600px) { :root { --gap: 16 } } :root { --gap: 8 } .box { width: 50vw; padding: var(--gap) } After a resize the box keeps `padding: 8` until the next viewport change — and because `stylesFamily` hands one memoised object to every consumer, a component mounted after the rotation gets the stale value too. Both drains — the `Dimensions` change handler and `StyleCollection.inject` — now call one exported `drainObservableBatch`, which removes a member BEFORE running it so a re-notification re-enqueues rather than landing on an entry the iteration has passed. Two tests. The diamond is mutation-proven: with the previous `for (const effect of batch)` it observes `2,10` — the derived value from before the change — and `2,20` with the work list. The memo suite gains a conditional-dependency test (the first compute takes a branch that never reads B, and opening the gate still picks up B's CURRENT value, because `effect.run` registers and pulls in the same pass), and its change test now pins the recompute COUNT rather than asserting it grew — the loose form was satisfied by the per-read recompute being removed, so it passed on both sides and guarded nothing. --- .../native/observable-batch-drain.test.ts | 69 +++++++++++++++++++ src/__tests__/native/observable-memo.test.ts | 43 +++++++++++- src/native-internal/style-collection.ts | 5 +- src/native/reactivity.ts | 36 +++++++++- 4 files changed, 144 insertions(+), 9 deletions(-) create mode 100644 src/__tests__/native/observable-batch-drain.test.ts diff --git a/src/__tests__/native/observable-batch-drain.test.ts b/src/__tests__/native/observable-batch-drain.test.ts new file mode 100644 index 00000000..7fd4e5ba --- /dev/null +++ b/src/__tests__/native/observable-batch-drain.test.ts @@ -0,0 +1,69 @@ +import { + drainObservableBatch, + observable, + observableBatch, +} from "../../native/reactivity"; +import type { Effect } from "../../native/reactivity"; + +/** + * A batch drain must run every effect it collects, INCLUDING one re-notified while it drains. + * + * The drain used to iterate the batch `Set` directly, and iterating a Set does not revisit a member + * it has already passed. So an effect notified a second time — because a derived observable it + * depends on recomputed after the effect ran — was dropped. + * + * That was survivable only while every `get` recomputed: whenever the effect happened to run it + * pulled fresh values out of its dependencies. Once a derived observable memoises, the effect reads + * the value its dependency held BEFORE the recompute and nothing runs it again, so the stale value + * is permanent. This is the diamond that makes it observable: one source, two derived readers, one + * subscriber reading both. + */ + +test("an effect re-notified during a drain runs again", () => { + // The shape is a diamond where the DEPENDENT is notified before its dependency: the subscriber + // reads the source directly AND through a derived observable that also reads it. Subscribing to + // the source first puts the subscriber ahead of the derived one in the batch, so it runs while + // the derived value is still stale — and the derived one's later notification lands on a member + // the iteration has already passed. + // + // That is exactly `vw -> { stylesObs, rootVariables } -> stylesObs`: a style reaching a viewport + // unit directly through the unit resolver, and again through a root variable whose media query + // tests a width. + const source = observable(1); + const derived = observable((read) => read(source) * 10); + + const seen: string[] = []; + const subscriber: Effect = { + observers: new Set(), + run: () => { + seen.push(`${String(source.get())},${String(derived.get())}`); + }, + }; + + // Order matters: the direct edge first. + source.get(subscriber); + derived.get(subscriber); + + observableBatch.current = new Set(); + source.set(2); + drainObservableBatch(); + observableBatch.current = undefined; + + // The subscriber's LAST observation must see both at the new value. Dropping the re-notification + // leaves it holding the derived value from before the source changed — permanently, because + // nothing runs it again. + expect(seen.at(-1)).toBe("2,20"); +}); + +test("a drain with nothing collected is a no-op", () => { + observableBatch.current = new Set(); + expect(() => { + drainObservableBatch(); + }).not.toThrow(); + observableBatch.current = undefined; + + // And with no batch open at all. + expect(() => { + drainObservableBatch(); + }).not.toThrow(); +}); diff --git a/src/__tests__/native/observable-memo.test.ts b/src/__tests__/native/observable-memo.test.ts index 377d9f02..ad8a3496 100644 --- a/src/__tests__/native/observable-memo.test.ts +++ b/src/__tests__/native/observable-memo.test.ts @@ -13,7 +13,10 @@ import type { Effect } from "../../native/reactivity"; * and re-assigns, and subscribers are notified from there. */ -const subscriber = (): Effect => ({ observers: new Set(), run: () => undefined }); +const subscriber = (): Effect => ({ + observers: new Set(), + run: () => undefined, +}); test("a derived observable computes once per change, not once per read", () => { let computed = 0; @@ -43,12 +46,46 @@ test("a derived observable recomputes when a dependency changes", () => { }); expect(derived.get(subscriber())).toBe(2); - const afterFirstRead = computed; + expect(computed).toBe(1); source.set(5); + // Exactly one recompute for the change, and none for the read that follows it. `toBeGreaterThan` + // would be satisfied by the per-read recompute this commit removes, so it would pass on both + // sides and guard nothing. expect(derived.get(subscriber())).toBe(10); - expect(computed).toBeGreaterThan(afterFirstRead); + expect(computed).toBe(2); +}); + +test("a conditional dependency is picked up the first time the branch is taken", () => { + // The classic memo hazard: the first compute takes a branch that never reads B, so B is never + // registered. It holds here because `effect.run` re-executes the read function and PULLS B's + // current value in the same pass that registers it — registration and the correct read are the + // same act, so nothing waits on a notification that was never owed. + let computed = 0; + const gate = observable(0); + const hidden = observable(100); + const derived = observable((read) => { + computed += 1; + return read(gate) > 0 ? read(hidden) : -1; + }); + + expect(derived.get(subscriber())).toBe(-1); + expect(hidden.observers.size).toBe(0); + + // A change to the unread branch cannot matter, and must not recompute. + hidden.set(200); + expect(derived.get(subscriber())).toBe(-1); + + // Opening the gate must pick up the CURRENT value of the branch it now reads. + gate.set(1); + expect(derived.get(subscriber())).toBe(200); + expect(hidden.observers.size).toBeGreaterThan(0); + + // And it tracks from then on. + hidden.set(300); + expect(derived.get(subscriber())).toBe(300); + expect(computed).toBeGreaterThan(1); }); test("a static observable is unaffected", () => { diff --git a/src/native-internal/style-collection.ts b/src/native-internal/style-collection.ts index eff34009..b4ff7f38 100644 --- a/src/native-internal/style-collection.ts +++ b/src/native-internal/style-collection.ts @@ -8,6 +8,7 @@ import type { import { DEFAULT_CONTAINER_NAME } from "../native/conditions/container-query"; import { + drainObservableBatch, family, observable, observableBatch, @@ -95,9 +96,7 @@ globalThis.__react_native_css_style_collection ??= { } } - for (const effect of observableBatch.current) { - effect.run(); - } + drainObservableBatch(); observableBatch.current = undefined; }, diff --git a/src/native/reactivity.ts b/src/native/reactivity.ts index 8d864a23..4d073cd5 100644 --- a/src/native/reactivity.ts +++ b/src/native/reactivity.ts @@ -136,6 +136,38 @@ export function observable( return obs; } +/** + * Run every effect a batch collected, including any re-notified while it drains. + * + * A batch is a `Set`, and iterating one does not revisit a member already passed — so an effect + * notified a SECOND time during the drain, because a derived observable it depends on recomputed + * after it ran, was silently dropped. That was survivable while every `get` recomputed: the effect + * pulled fresh values out of its dependencies whenever it happened to run. It is not survivable + * once a derived observable memoises, because the effect then reads the value its dependency held + * before the recompute, and nothing runs it again — the stale value is permanent. + * + * Draining as a work list fixes it: the member is removed BEFORE it runs, so a re-notification + * re-enqueues it rather than landing on an entry the iteration has already passed. + */ +export function drainObservableBatch() { + const batch = observableBatch.current; + + if (!batch) { + return; + } + + while (batch.size > 0) { + const next = batch.values().next(); + + if (next.done) { + break; + } + + batch.delete(next.value); + next.value.run(); + } +} + export function cleanupEffect(effect: Effect) { if (!effect) return; for (const dep of effect.observers) { @@ -278,9 +310,7 @@ Dimensions.addEventListener("change", ({ window }) => { vw.set(window.width); vh.set(window.height); - for (const effect of observableBatch.current) { - effect.run(); - } + drainObservableBatch(); observableBatch.current = undefined; }); From 29db6bc592cb94f8fdac6e784f27226de26beb16 Mon Sep 17 00:00:00 2001 From: Yevhenii Date: Thu, 20 Aug 2026 02:22:15 +0300 Subject: [PATCH 6/7] test(native): pin that the nativeStyleMapping drain is idempotent on a cached entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `nativeStyleMapping` drains keys OUT of the resolved style object and writes them onto real props — `delete source[key]`, then `props[path] = value`. It mutates. That was harmless while every read of the styles observable recomputed: each render got a fresh object and the mutation died with it. The memo makes the object it mutates the CACHED one, shared by every consumer of that entry and surviving every render. It is idempotent — the second pass finds the key already drained and continues — and every shape measures correct. But nothing enforced that, and "it happens to be a no-op" is not a property to leave resting on an accident while the memo makes it load-bearing. Two tests, on the one shipped component with a `target: false` mapping: ten re-renders read the same mapped value, and a second component joining the same cache entry sees it too. A drain that stopped being idempotent would surface as a MISSING prop on the second consumer rather than a wrong one, which is the harder failure to notice. --- .../native-style-mapping-idempotence.test.tsx | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 src/__tests__/native/native-style-mapping-idempotence.test.tsx diff --git a/src/__tests__/native/native-style-mapping-idempotence.test.tsx b/src/__tests__/native/native-style-mapping-idempotence.test.tsx new file mode 100644 index 00000000..bce343f1 --- /dev/null +++ b/src/__tests__/native/native-style-mapping-idempotence.test.tsx @@ -0,0 +1,98 @@ +import { Button as RNButton, type ButtonProps } from "react-native"; + +import { render } from "@testing-library/react-native"; +import { copyComponentProperties } from "react-native-css/components/copyComponentProperties"; +import { registerCSS, testID } from "react-native-css/jest"; +import { useCssElement } from "react-native-css/native"; +import type { + StyledConfiguration, + StyledProps, +} from "react-native-css/runtime.types"; + +/** + * `nativeStyleMapping` drains keys OUT of the resolved style object and writes them onto real props + * — `delete source[key]`, then `props[path] = value`. It mutates. + * + * That was harmless while every read of the styles observable recomputed: each render got a fresh + * object and the mutation died with it. Now the observable memoises, so the object it mutates is the + * CACHED one, shared by every consumer of that entry and surviving every render. + * + * It is idempotent — the second pass finds the key already drained and continues — but nothing + * enforced that, and "it happens to be a no-op" is not a property anyone can rely on while editing + * the drain. This pins it: the same component re-rendered many times, and a second component + * joining the same cache entry, must both see the mapped prop and never a re-drained style. + */ + +const mapping: StyledConfiguration = { + className: { + target: false, + nativeStyleMapping: { color: "color" }, + }, +}; + +const Button = copyComponentProperties( + RNButton, + (props: StyledProps) => + useCssElement(RNButton, props, mapping), +); + +test("re-rendering does not re-drain the cached style object", () => { + registerCSS(`.drain-once { color: orange; }`); + + const tree = render( +