fix(native): share the :root variable registries across a dual-package split - #410
Open
YevheniiKotyrlo wants to merge 2 commits into
Open
Conversation
`native-internal/root` created its two registries at module scope, so two
copies of the module meant two independent stores.
The package's `exports` map splits `import` and `require` onto different
builds and Metro resolves that condition per REQUESTING module, so a
compiled-CommonJS dependency and first-party source bind different copies.
A `:root` variable injected into one was invisible to the other, and the
value silently fell back to its seed — a themed class rendered React
Native's default rather than the theme's colour, with no error.
Every other stateful module here already guards against this
(`style-collection.ts`, `variables.tsx`); these two did not.
Creation and seeding are ONE step, behind one global. A `??=` on the
registries alone would leave the seeds running unconditionally, so a copy
initialising AFTER the stylesheet inject would re-run `set([[14]])` and
clobber a project's own `:root { font-size: 16px }` back to 14 — silently
rescaling every rem-derived value to 87.5%. Both registries share one global
for the same reason: two globals could be half-initialised.
The guard is a named `resolveRootVariableRegistries()` so a second copy's
behaviour is reachable from a test without re-evaluating the module —
`await import()` and `require()` are both unavailable here (no
`--experimental-vm-modules`, and the lint config forbids the latter).
3 tests, each mutation-proven to go red when the guard is removed.
The guard now sits at module scope in the shape style-collection.ts and variables.tsx already use, so `resolveRootVariableRegistries` is gone from `react-native-css/native-internal`'s public surface. It was exported so a test could reach a second copy's behaviour, and that is not needed: `jest.resetModules()` plus a re-import evaluates the module again against the same globalThis, which is the dual-package case exactly. The test that replaces it holds an injected rem of 16 across that second evaluation, and goes red when the seeds are moved outside the guard — the regression the test it replaces named but did not catch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When a bundle contains both builds —
dist/modulefor ESM importers anddist/commonjsfor CommonJS ones — the two copies get separate:rootvariable registries. The compiled stylesheet is injected exactly once, into whichever copy the app's.cssmodule resolved. Every component resolving through the other copy then looks upvar(--anything)in an empty registry, getsundefined, and silently renders with no value.For a
classNamecolour that means the element falls back to React Native's default text colour — black on Android, invisible on any dark surface. Nothing errors; the class is found (the style registry IS shared), only its value resolves to nothing.This reproduces in a stock Expo SDK 57 app for any component shipping compiled CommonJS.
expo-router's<Link>is the case that surfaced it.Measured
Android, dark theme,
text-danger-foreground(#fb2c36), sampled from the rendered pixels rather than by eye:Both builds are genuinely in the graph — resolving each module's dependency map to its path in a real Metro dev bundle:
Root cause
src/native-internal/root.tscreates its registries at module scope, so two copies of the module means two independent registries.exportssplitsimport/requireacross builds and Metro resolves that condition per requesting module.Two sibling modules already guard against exactly this —
native-internal/style-collection.tsandnative-internal/variables.tsboth useglobalThis.… ??=.root.tswas missed.Why the one-line
??=is a regressionroot.tsdoes not only create the registries, it seeds them (__rn-css-rem, and the platform__rn-css-color).__rn-css-remis also set by the injected stylesheet — a project pinning:root { font-size: 16px }injects[[16]]. So a bareglobalThis.x ??= rootVariableFamily()leaves the seeds running unconditionally, and a second copy initialising after the inject re-runsset([[14]])and clobbers 16 back to 14 — silently rescaling every rem-derived utility to 87.5%.Creation and seeding therefore have to be one atomic, once-only step, which is what this does.
Tests
Three added, each mutation-proven (I broke the fix and watched the assertion go red before trusting it). Suite, typecheck and lint sit at the pristine-
mainbaseline.