fix(native): make colorScheme.set move every reader of the scheme - #415
fix(native): make colorScheme.set move every reader of the scheme#415YevheniiKotyrlo wants to merge 7 commits into
Conversation
`colorScheme.set()` moved only this library's observable, so the class layer and React Native's own readers disagreed. `useColorScheme()` and every prop-valued colour read `Appearance`; `dark:` utilities read the observable. An app calling the documented setter moved one and not the other, and rendered a light canvas under dark chrome. Writing both in the one call is the whole fix. It does not try to make a direct `Appearance.setColorScheme()` visible to the class layer: that writer emits no event, and the class layer is push-based, so nothing short of a notification can move an already-mounted element.
There were two sources of truth for the scheme with different null semantics. `colorScheme.get()` coalesces through Appearance to a definite value; the class layer read the raw observable. The observable holds null at rest and after `set(null)`, so `prefers-color-scheme: light` and `dark` both failed while `get()` reported light — the element fell through to its unconditional rule. That is the same two-readers-disagree defect this branch is named for, one function along, and it is reachable through the setter the branch just changed. The tests are rewritten around what each one actually pins. The repaint case duplicated media-query.test.tsx byte for byte and is gone; the OS-event case stays, relabelled as the guard it is for Appearance.addChangeListener. The write-through assertion now checks the argument rather than the resulting cache, which passed under every mutation because get() falls back to Appearance. The fixture is three-way so "matched neither branch" is distinguishable from "matched light" — the failure above is invisible to a two-colour fixture.
The write-through moved two of the three readers. React Native's
setColorScheme assigns Appearance's cache and calls the native module;
the only eventEmitter.emit("change") in Libraries/Utilities/Appearance.js
sits inside the native `appearanceChanged` handler. So a write the
platform does not echo back moves getColorScheme() and notifies nobody —
and every documented way to track the scheme, useColorScheme included, is
useSyncExternalStore over addChangeListener.
colorScheme.set now announces the change on the same device event the
platform uses, so Appearance itself performs the cache write and the emit
exactly as it does for an OS change. DeviceEventEmitter is a public
react-native export and `appearanceChanged` is the event Appearance
subscribes to through NativeEventEmitter, which registers on that same
emitter.
Guarded on the cache having actually moved, so this reports a change and
never invents one: where there is no native Appearance module the write
is a no-op and both reads are null, and a redundant set of the scheme
already in force stays silent, matching the observable's own equality
guard.
The suite now fakes Libraries/Utilities/NativeAppearance rather than
replacing Appearance itself, so the cache, the change event, the
"unspecified" coercion and their ordering are react-native's own instead
of a transcription of them. That is what lets the OS-change test drive
the real platform path, and what makes the subscriber claim measurable
rather than argued.
Deriving the announcement from Appearance's cache made it depend on the
one expression react-native changed at 0.86. `setColorScheme` writes that
cache from the requested value on 0.86+; before it, from
`toColorScheme(NativeAppearance.getColorScheme())` — a read-back that is
stale on both platforms, because Android posts the night-mode switch to
the UI thread through `UiThreadUtil.runOnUiThread` (`postDelayed(r, 0)`)
and iOS never assigns `_currentColorScheme` in `setColorScheme:`. So the
cache-derived guard suppressed its own emit on every react-native below
0.86 and left subscribers to the platform echo, and on 0.86 it broadcast
`{colorScheme: null}` for `set(null)` — a scheme no reader can render.
The announcement now carries what the caller asked for, so it says the
same thing on every version in the declared peer range, and only a
resolved scheme is announced. Every other member of ColorSchemeName is a
hand-back rather than a scheme — null and undefined before 0.86, the
literal "unspecified" from 0.86 on — and only the OS knows what one
resolves to; its own echo delivers that.
Two suites cover the two cache-write rules, both driving the setter
against a platform that applies the write on a later turn, through an
explicit flush seam rather than a timer. The 0.81 suite runs the
installed Appearance over an asynchronous NativeAppearance; the 0.86 one
transcribes the four functions of that version's Appearance.js, which
cannot be installed beside it, and keeps react-native's own
NativeEventEmitter registration so what reaches its cache is what
reaches the real one.
`colorScheme.get()` and the `prefers-color-scheme` evaluator both resolved what
the scheme channel holds with `?? Appearance.getColorScheme() ?? "light"`. That
chain fires only on a nullish value, and `"unspecified"` — 0.86's spelling of
"follow the system", where 0.81 spells `null` — is not nullish. It passes
straight through.
A reader handed the literal matches neither `prefers-color-scheme: dark` nor
`: light`, so an app that asks to follow a dark system loses every
scheme-conditional class rather than falling back to one. On Android nothing
repairs that until the user toggles the system theme, because AppearanceModule
emits only when the resolved scheme changes.
`resolveColorScheme` accepts a resolved scheme and rejects everything else,
rather than naming the members it must reject. That is what makes it total: a
future release can add another "no scheme yet" spelling and it keeps answering
correctly, where a deny-list would silently gain a third hole. Both readers call
it, because the class layer and the prop layer disagreeing about the scheme is
the defect — the two copies it replaces had already drifted into being wrong
together.
The existing `set('unspecified')` test stepped past the window, asserting only
after the platform echo repairs it. The new test asserts inside it.
|
Cross-referencing #415 and #429 — they merge with zero conflicts and the result does not compile. Flagging it now, because nothing will warn whoever lands the second one.
The two halves are each correct alone:
On the merge, #429's wider union reaches a call react-native 0.81 types as Neither PR can pre-empt this, which is why it is a note rather than a fix on one of them. #415 cannot import The resolution is one line at merge time: route the native setter through the same seam the web one already uses, rather than calling // src/native/api.tsx
import { setAppearanceColorScheme } from "../color-scheme";
…
setAppearanceColorScheme(value); // instead of Appearance.setColorScheme(value)Measured: that takes the merged tree from one error to zero. The seam exists precisely because the one member the supported react-native range declares incompatibly is the "follow the system" write, so this is what it was built for. Worth pairing with a compile-level guard, since the failure is a type error rather than a behaviour: the merged tree must typecheck. A runtime twin already exists on #429 at |
…tarts at These comments say 0.86 in four places. Measured across every react-native in the Yarn cache, the break starts at 0.82.0 — four minor versions earlier: 0.81.2 / 0.81.4 / 0.81.5 ColorSchemeName = 'light' | 'dark' | null | undefined 0.82.0 … 0.86.0 ColorSchemeName = 'light' | 'dark' | 'unspecified' The runtime moves in the same release. 0.81.5 coerces the nullish request — `NativeAppearance.setColorScheme(colorScheme ?? 'unspecified')` — and 0.82.0 passes the argument through verbatim. nativewind#429 already names 0.82 on its own surface, so left alone the two branches would land in one tree disagreeing about one boundary. The cache-write sentence gets rewritten rather than renumbered, because the range has three states and not two: before 0.82 the cache is a read-back of the native module, from 0.82 it is the requested value, and later in the range `"unspecified"` is resolved against the OS before being stored. Scoping the sentence to a RESOLVED scheme collapses the last two — the only value they disagree about is `"unspecified"`, which is exactly the value the guard below declines to announce — and it keeps the comment from naming a boundary this measurement cannot place: the cache holds 0.84.1 and 0.85.3 but not 0.85.0 through 0.85.2, so the second transition is bounded only to (0.84.1, 0.85.3]. Comments only; no behaviour changes. `reactivity.ts:229` is left alone — "the request 0.81 spells `null`" is a statement about 0.81 and is correct.
|
Following up on the cross-reference above with two measured additions, one of which is a trap worth naming explicitly. An inline cast at the call site compiles, and is the wrong resolutionThe obvious shortcut for the merge break is to widen the argument in place rather than route through the seam: Appearance.setColorScheme(value as Parameters<typeof Appearance.setColorScheme>[0]);That does take the merged tree to zero type errors, on both react-native ends of the peer range. It is still wrong, because it silences both arms of a union that disagrees for a reason: Up to 0.81.5 a nullish request is coerced to That is the failure #415's own comment forbids in the same function: "Announcing it would put a value in Appearance's cache that no reader can render." The boundary is 0.82.0, not 0.86Measured across every react-native in the Yarn cache, by extracting The type narrowing and the loss of the One nuance worth recording for whoever touches this next, because it is easy to collapse into the same number: there is a third state later in the range, where A latent one, for #415#415's three new colour-scheme test files do not type-check against ≥0.82 — |
…es into one The header attributed to 0.86 a rewrite that landed at 0.82, and then used that number to introduce an expression which is a second, later change. Two boundaries, one version number, and the quoted code belongs to the later one. Measured across the versions available: `setColorScheme` stops reading the cache back and writes the requested value at 0.82.0, and the read-back returns for the literal "unspecified" alone by 0.85.3 — present there, absent at 0.84.1. The header now says "by 0.85.3" rather than naming a start version, because 0.85.0 through 0.85.2 could not be read and at-or-before 0.85.3 is what the evidence supports. The citation at the foot of the quote is untouched: 0.86.0 does ship that expression, and the file models 0.86 deliberately. Comments only.


Problem
colorScheme.set()moves this library's observable and nothing else, so the two halves of an app's theming disagree.The class layer (
dark:utilities,@media (prefers-color-scheme)) reads the observable. React Native's own readers —useColorScheme()and every prop-valued colour — readAppearance. An app calling the documented setter moves the first and not the second, and renders a light canvas under dark chrome.This is a native-only change, and "every reader" is a claim about the native runtime.
src/web/api.tsxalready intends to write through toAppearance, and on web that call is a hardTypeError— a pre-existing defect this deliberately does not close, described in full in the last section.Fix
Three parts, all about the same thing — the library had two sources of truth for the scheme, and moving one of them told nobody.
The setter reaches all three readers, which are three separate channels: the class layer reads the observable,
useColorScheme()readsAppearance's cache, and every store built the documented way is subscribed throughAppearance.addChangeListener.And the class layer resolves the scheme the way
colorScheme.get()already does.get()coalesces throughAppearanceto a definite value;conditions/media-query.tsread the raw observable. That observable holdsnullat rest and afterset(null), soprefers-color-scheme: lightanddarkboth failed whileget()reportedlight, and the element fell through to its unconditional rule. Same defect as above, one function along, and reachable through the setter this PR changes:Why the announcement goes through
DeviceEventEmitterBecause that is the channel
Appearanceitself listens on, and React Native provides no other way in.Libraries/Utilities/Appearance.jshas two write paths and only one of them emits.setColorSchemeassignsstate.appearanceand calls the native module; the soleeventEmitter.emit('change', …)sits inside theappearanceChangedhandler registered ingetState(). So a write the platform does not echo back movesgetColorScheme()and notifies nobody — anduseColorSchemeisuseSyncExternalStore(addChangeListener, getColorScheme), so with no event it is never told to re-read. Neither is any other store built the same way, which is every documented way to track the scheme.That handler is registered through
new NativeEventEmitter(NativeAppearance).addListener('appearanceChanged', …), andNativeEventEmitter.addListenerregisters onRCTDeviceEventEmitter— its own comment says so ("all native events are fired via a globalRCTDeviceEventEmitter").DeviceEventEmitteris that emitter, exported from thereact-nativeroot (index.js, andtypes/index.d.ts). So emitting there hands the value toAppearance, which performs its own cache write and its ownchangeemit, exactly as it does for an OS change. The library invents no notification and reimplements none ofAppearance's behaviour.Why it carries the requested scheme rather than a read of the cache
Because
setColorSchemewrites that cache differently on either side of react-native 0.86, and the declared peer range (react-native >= 0.81) spans the change.Before 0.86 that read-back is stale on both platforms, because the platform applies the write on a later turn:
AppearanceModule.setColorSchemewraps the night-mode switch inUiThreadUtil.runOnUiThread {}, andrunOnUiThreadismainHandler.postDelayed(runnable, 0): always posted, never run inline.getColorScheme()still answers from the configuration in force.RCTAppearance.mm'sgetColorSchemereturns_currentColorScheme, which is assigned atinitand insideappearanceChanged:and never bysetColorScheme:— that method setswindow.overrideUserInterfaceStyleand nothing else.So a guard that reads the cache back concludes "nothing moved" on every react-native below 0.86 and suppresses its own emit; subscribers are left to the platform echo, which is exactly where they were before. From 0.86 the same guard fires — including on
set(null), where the cache goesnulland the announcement broadcasts{colorScheme: null}to every subscriber, tellinguseColorScheme()the app has no scheme at all.Keying on the requested scheme takes the version out of the question: the announcement says the same thing on every react-native in the peer range, because it never asks what the platform did with the write.
Only a resolved scheme is announced. Every other member of
ColorSchemeNameis a hand-back rather than a scheme, and that type moved at the same release — 0.81 declares'light' | 'dark' | null | undefined, 0.86 declares'light' | 'dark' | 'unspecified', so on the current release"unspecified"is the type-legal way to hand the scheme back andnullis not in the type at all. Only the OS knows what a hand-back resolves to. Announcing the request itself would put a value inAppearance's cache that no reader can render, and on 0.81"unspecified"tripstoColorScheme's invariant outright; the platform's own echo delivers the resolved scheme instead, exactly as it does for an OS change.previousis what keeps a set of the scheme already in force silent.The guard is well-typed on both, which is worth saying because
ColorSchemeNameinverted at the same release: comparing against the two scheme literals narrows identically under'light' | 'dark' | null | undefinedand under'light' | 'dark' | 'unspecified', so the line adds no error to atscrun on either.yarn typecheckhere runs against the 0.81 pin; the 0.86 union was checked by replaying the expression against it directly.Nothing in
src/compileris touched or reachable. The compiler emits the["=", "prefers-color-scheme", "dark"]condition tuple and never resolves it — its only import fromreact-nativeis thePlatformOSTypetype — so both halves of this change are confined to the native runtime that evaluates the tuple.One notification per
colorScheme.set, and the guard is not what bounds it. The emit drivesAppearance'schange, which drives this library's own subscription (reactivity.ts:222), which callscolorSchemeObs.set(event.colorScheme)with the value the line above already wrote — andobservable.set'sObject.isearly return (reactivity.ts:73-93) is what makes that second write notify nobody. Removing the guard entirely still terminates cleanly (column C below): it announces redundantly, it does not recurse.The real home for this is
Appearance.setColorSchemeitself, which should announce a cache it just moved. This is the library-side close; I am happy to take it upstream to react-native as well if you would rather have it there.Known limits, precisely
A platform that echoes the write back delivers two
changeevents with the same value. Measured as["dark", "dark"]for onecolorScheme.set("dark")followed by the echo, in both new suites. iOS setsoverrideUserInterfaceStyle, which fires a trait changeRCTAppearancere-emits; Android'ssetDefaultNightModereachesonConfigurationChanged. Both dedup against their own last emission, and neither can see aDeviceEventEmitter.emitmade in JS, so the echo still arrives.useSyncExternalStorebails on an identical snapshot, souseColorSchemere-renders once and any other subscriber sees an idempotent repeat. React Native makes no dedup guarantee on this event in the first place.A hand-back followed by a set of the scheme the stale cache happens to hold announces nothing. Measured against the 0.86 model:
set("dark"), thenset("unspecified"), thenset("light")before the platform echo lands, leaves subscribers on"dark"whileAppearance.getColorScheme()already reads"light".previousis read from a cache thatsetColorScheme's own read-back can move without notifying anybody, so it is not reliably what subscribers last heard. Closing it means the setter tracking what it last announced — state it does not otherwise need — so I have left it open rather than add that unasked.I have not measured this on hardware. The mechanism is read off react-native's own source (0.81.4 and 0.86.0, JS and both native modules) and measured in the suite; not on a device.
A direct
Appearance.setColorScheme()still does not move an already-mounted element. That is unchanged by this PR and is a different defect: the observable is seeded once at import and never re-read, so a fresh mount after a direct write is stale too — no notification would fix that, and no pull would either. Making it work means the class layer subscribing toAppearancerather than mirroring it, which is a change to how the observable is constructed and a separate question from this one.Reading
Appearancethrough on every observableget()is the obvious way to try, and it is a trap worth recording:get()on a function-init observable assigns the cached value without notifying, whilerun()'s equality guard compares against that same cache. A read landing between a change and its notification swallows the notification, permanently. I measured that as two elements with the same class rendering different colours. It does not apply to the observable as it stands — seeded with a value, it is static — but it is why the read-through is not the shortcut it looks like.Tests
Eighteen, across three files. Each column below reverts or alters one half of the change; every test is killed by at least one, except the last, which is the floor by design.
color-scheme-appearance.test.tsxuseColorSchemeis movesget()set(null)hands the scheme backcolor-scheme-appearance-async.test.tsxset(null)announces no scheme of its owncolor-scheme-appearance-rn-0-86.test.tsxset(null)broadcasts no null schemeset('unspecified')broadcasts no literalColumn D is a cache-derived announcement, which is what makes the read-back dependency measurable rather than argued: it passes every test in the original suite and fails four here. Column E is the same guard without the resolved-scheme test — the one mutation that reaches only the 0.86 literal.
The 0.81 suites fake
Libraries/Utilities/NativeAppearance, notAppearance. Under the jest presetTurboModuleRegistry.get("Appearance")is null, so the real module takes its absent-native branch — every readnull,setColorSchemea no-op, and noappearanceChangedlistener registered — which is why it cannot express the behaviour under test. Faking the one module it is missing leaves the realAppearance.jsrunning, so its cache, itschangeemit, itsunspecifiedcoercion and their ordering are react-native's own rather than a transcription of them.color-scheme-appearance-async.test.tsxis that same instrument over a native module that applies the write on a later turn, through an explicit flush seam rather than a timer, which is what a device does.color-scheme-appearance-rn-0-86.test.tsxis the one file that transcribes rather than runs: 0.86'sAppearance.jscannot be installed beside the 0.81 this repo pins, so its four functions are transcribed and quoted, and theappearanceChangedregistration is still react-native's ownNativeEventEmitter— what reaches that cache is what reaches the real one. Transcribing the version that cannot be installed, and only that, is the whole of the fakery.The write-through test asserts the argument passed to
setColorScheme, not the resulting cache. Asserting the cache passes under every mutation, becauseget()falls back toAppearance.getColorScheme()and either writer alone satisfies it.The subscriber tests render
useSyncExternalStore(addChangeListener, getColorScheme)— the exact shape ofuseColorScheme, which cannot itself be used becausereact-native/jest/setup.jsreplaces it withjest.fn(() => "light").The
prefers-color-schemefixture is three-way — unconditional green,lightblue,darkred — so "matched neither branch" is distinguishable from "matched light". Theset(null)defect is invisible to a two-colour fixture.Full suite on Windows, stable across four runs:
2 failed, 4 skipped, 56 passed, 58 of 62 totalsuites and3 failed, 21 skipped, 1067 passed, 1091 totaltests, against56 of 60/1057 passed, 1081 totalbefore this commit — exactly the ten new tests, no change in failures. The three that fail are thesrc/__tests__/babel/*path suites, unrelated to this change and fixed by #390.yarn typecheckandyarn lintboth exit 0.Second commit —
"unspecified"was leaking through the same resolutionThe first commit makes this package's own
setmove every reader. Reviewing it turned up a second hole in the resolution it leans on, reproducible on its own.Both readers resolved the scheme with
?? Appearance.getColorScheme() ?? "light". That chain fires only on a nullish value, and"unspecified"is not nullish — it is 0.86's spelling of "follow the system", where 0.81 spellsnull. So the literal passes straight through to a reader.A reader handed it matches neither
prefers-color-scheme: darknor: light, so an app that asks to follow a dark system loses every scheme-conditional class rather than falling back to one. On Android nothing repairs that until the user toggles the system theme, becauseAppearanceModuleemits only when the resolved scheme changes.The existing
set('unspecified')test could not see it: it asserts only after the platform echo, which repairs the value. The new test asserts inside that window, and fails on the parent commit withExpected: not "unspecified".resolveColorSchemeaccepts a resolved scheme and rejects everything else, rather than naming the members it must reject. That is what makes it total — a future release can add another "no scheme yet" spelling and it keeps answering correctly, where a deny-list would silently gain a third hole. It is also why nothing in it compares against"unspecified", which is outside theColorSchemeNamethe pinned react-native declares.Both readers call the one function, because the class layer and the prop layer disagreeing about the scheme is the defect itself — and the two copies it replaces had already drifted into being wrong together. The
media-query.tscopy even carried the comment "The same resolution the publiccolorScheme.get()uses", which is the invariant this makes structural instead of conventional.Note
This changes the observable behaviour of a public API, which
CONTRIBUTING.mdasks be discussed in an issue first. Happy to move it to one if you would rather — I opened it as a PR because the change and its reproduction are easier to read as a diff.Separately, the web half of this API is broken
src/web/api.tsx:76callsAppearance.setColorScheme(name), andreact-native-web@0.21.1does not implement it — itsAppearanceexports exactlygetColorSchemeandaddChangeListener. So that call is aTypeErrorfor the first caller.Nothing in the repo can see it:
src/web/api.tsx:8importsAppearancefrom"react-native", so TypeScript resolves RN's.d.ts, which does declaresetColorScheme, and the swap to react-native-web happens at bundler resolution. There are no runtime tests undersrc/web/**at all. It predates this change (aeb0085).I have deliberately not fixed it here, because it is not a missing line — it is a missing concept. A browser will not let JavaScript override
prefers-color-scheme; react-native-web has nosetColorSchemebecause there is nothing for it to do. So closing it means deciding whatcolorScheme.setmeans on web, and there are three different answers:@media (prefers-color-scheme), sodark:follows the library rather than the browser. That is a feature, not a fix.That is your call rather than mine, and it changes what a public API promises on a platform this PR does not otherwise touch — so it wants its own PR and probably its own issue. Happy to send whichever of the three you want.