Repository navigation
fix(native): make colorScheme.set move every reader of the scheme - #415
YevheniiKotyrlo wants to merge 7 commits into
Conversation
|
Outdated: this compared the August heads of #415 and #429. #415 is now re-written onto The original noteCross-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 |
|
Outdated: this compared the August heads of #415 and #429. #415 is now re-written onto The original noteFollowing 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 — |
colorScheme.set moved this library's observable and nothing else, so the class layer and React Native's own readers disagreed: useColorScheme() and every store built on Appearance.addChangeListener kept the old scheme. The setter now writes through to Appearance and announces the scheme on the appearanceChanged device event, because Appearance.setColorScheme writes its cache without notifying a listener, and that event is the one Appearance itself handles. It announces only a resolved scheme it changes. A follow-the-system request is handed to Appearance as "unspecified"; on react-native 0.82.0-0.84.1, which caches that request verbatim, the scheme in force before the hand-back is put back in the cache. colorScheme.get() and prefers-color-scheme resolve the held value through one function, which answers a scheme for any value that is not one: the reported scheme, then light (MQ5 §5.4). The class layer read the raw observable, so a held null or request matched neither scheme.
eac5a41 to
79fc5d5
Compare
…without its module
…ce double read-only
…est reset a function Appearance answers null at import only without its native module, so a request still reaches the platform after app code hands Appearance back with null, which 0.82 and later cache as it came. The jest reset is resetBetweenTests, which its test calls after changing the scheme, and the scheme tests claim of the library's own layers only what holds before the platform reports.


Problem
On
main,colorScheme.set()moves this library's observable and nothing else:useColorScheme()and every store built onAppearance.addChangeListenerkeep the old scheme. An app calling the setter therefore renders itsdark:classes on one scheme and its prop-valued colours on the other.nullafter a follow-the-system request.prefers-color-scheme: lightanddarkthen both fail whilecolorScheme.get()reports a scheme, and the element falls through to its unconditional rule.Solution
set()hands the request toAppearance.setColorScheme, which holds it on the platform, so it moves every reader the way the documentedAppearance.setColorSchemedoes. The class layer follows the schemeAppearancereports and announces none of its own.Appearancenative module, as in a jest suite, nothing else holds a scheme, soset()keeps it here, as onmain. Whether the module exists is read once at import, whereAppearance.getColorScheme()answersnullonly without it, so a request still reaches the platform after app code handsAppearanceback withnull, which 0.82 and later cache as it came.colorScheme.get()andprefers-color-schemeresolve the value throughresolveColorScheme: one that names no scheme answerslight, as MQ5 §12.5 answers no preference.react-native-css/jest's reset,resetBetweenTests()insrc/jest/reset.ts, starts each test from the schemeAppearancereports.Tests
color-scheme-appearance.test.tsxruns the installed React Native 0.86Appearanceover a device double that applies an override after the call returns and sendsappearanceChangedonly when its scheme moves, as iOS and Android do: iOS applying one turn's overrides together, Android each in turn, each with a light and a dark system scheme. It covers:Appearance, with no event of the library's owncolorScheme.get()keeping the scheme in force until the platform applies a request, then every layer taking itnulland through"unspecified"AppearanceitselfAppearanceback withnullcolor-scheme-without-appearance-module.test.tsx: without a native module,set()holds the scheme and a hand-back resolveslighton both layers.color-scheme-jest-reset.test.tsx: the jest helper's reset puts back the schemeAppearancereports after a test changed it.main, 38 of the 49 cases fail; all pass here.Verification
Every
ci.ymljob passes on this head merged intomainwith #477's harness fix: lint, typecheck, the tests (1396 passed), both builds, the web export, nothing unstaged and commitlint, and the iOS dev build on macOS 27.0.1 with Xcode 27.0, where CI pins 26.6.Device evidence, before and after: #415 (comment)
Known limits
colorScheme.get()follow the scheme the platform reports, so right afterset()they keep the scheme in force until the platform applies the request. React Native 0.82 to 0.87 cache the request inAppearance.getColorScheme()at the call, so anAppearancesubscriber that renders before the report reads the request.uiModeitself, aReactFragmenthost, and an iOS app that sets the scheme while in the background, until React Native reports every scheme it applies.Appearancebefore this library loads, the class layer answerslightuntil the platform next reports a scheme; and whereset(null)moves no scheme,Appearance.getColorScheme()answers'unspecified'until then, which react/react-native#54993 fixed in 0.83.5 and 0.85.colorScheme.setthrows onmain, since react-native-web 0.21'sAppearancehas nosetColorScheme; fix(web): set the color scheme on the root element, plus coverage #428 puts the scheme on the root element there.Merge order
prefers-color-schemelookup into one feature lookup. Whichever lands second resolves the scheme there.prefers-reduced-motionbeside theprefers-color-schemecase, and its ownresetBetweenTests()insrc/jest/reset.ts, which resets the scheme through the publiccolorScheme. Whichever lands second keeps both resets' lines in one function, the held scheme reset as this PR resets it, since the publicset()now asksAppearanceand would leave the class layer as it was.colorSchemeinto reactivity's pinned process-global state. Whichever lands second readshasAppearanceModulewhere that state is created, since a second copy of the module can load after app code handsAppearanceback withnull.setAppearanceColorScheme, the write that type-checks across the peer range, and acolor-scheme.test.tsxcase whose native-module double never reports a scheme. Whichever lands second hasset()write through it and that double report the scheme it applies.inherits: falseon registered custom properties #412 and fix(native): measure em against the font size each element computes #498 add lines to the reset this PR moves intosrc/jest/reset.ts, and fix(types): type CSS custom properties against what both platforms honour #423, fix(native): a custom property with no value produces no key #431, fix(native): answer :dir() per element, and compile every spelling of it #459, fix(compiler): read a media query's em and rem against the font size rem inlines #494 and fix(web): take the variable name the native variable hooks take #496 change lines beside this PR's in the same files, so whichever lands second resolves them.