Repository navigation
fix(native): make colorScheme.set move every reader of the scheme - #415
YevheniiKotyrlo wants to merge 1 commit 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 — |
eac5a41 to
79fc5d5
Compare
colorScheme.set() moved this library's observable alone, so useColorScheme() and every Appearance.addChangeListener store kept the old scheme, and after a follow-the-system request the class layer held null and matched neither prefers-color-scheme: light nor dark. set() hands each request to Appearance.setColorScheme, and the class layer follows the scheme the platform reports through Appearance. Until the platform has reported one, what Appearance answers after the latest request stands for it, which without a native module, as in a jest suite, is the request itself. colorScheme.get() and prefers-color-scheme, on a rule and on a :root variable, resolve a value that names no scheme to light (MQ5 §12.5), and the jest reset starts each test from what Appearance answers.
59f737e to
43e9b80
Compare


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 a rule or a:rootvariable falls through to its unconditional value.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 scheme the platform reports throughAppearanceand announces none of its own.Appearanceanswers after this library's latest request stands for it; without a native module, as in a jest suite,Appearanceanswersnulland the request itself stands.colorScheme.get()andprefers-color-scheme, on a rule and on a:rootvariable, resolve throughresolveColorScheme, where a value 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 whatAppearanceanswers.React Native decides when the readers move:
AppearanceModule.setColorScheme) and iOS on the main queue (RCTAppearance), and reports it asappearanceChanged; until then the class layer andcolorScheme.get()keep the scheme the platform last reported. From 0.82,Appearance.setColorSchemecaches the request at the call without notifying a listener, so anAppearancesubscriber that renders before the report reads the request.uiModeitself, or to aReactFragmenthost, since only an activity'sonConfigurationChangedreachesAppearanceModule.onConfigurationChanged, nor one an iOS app sets in the background, whose trait changeRCTSurfaceHostingViewdrops. The class layer keeps the previous scheme there until the next report.'unspecified'on 0.82.0 to 0.83.4, 0.84.0 and 0.84.1, which react/react-native#54993 fixed in 0.83.5 and 0.85, and from 0.82 anullpassed against its type. Whereset(null)then moves no scheme,Appearance.getColorScheme()answers that value until the platform next reports; where it is whatAppearanceanswers when this library loads, the class layer answerslightuntil the platform reports a scheme or a request of this library's leavesAppearanceanswering one. A call toAppearance.setColorSchememade outside this library notifies no listener, so it reaches the class layer, as any subscriber, only through that report.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. Its cases cover requests, hand-backs throughnulland"unspecified", requests the platform has not applied yet, a late mount, an OS change, a scheme set throughAppearanceitself, and app code'snullhand-back made before this library loads or after.color-scheme-without-appearance-module.test.tsxholds the scheme without a native module, for a rule and for a:rootvariable;color-scheme-jest-reset.test.tsxthe reset;compiler/media-query.test.tsthe conditions as compiled.main, 56 of the 67 cases fail; all pass here.Verification
Every
ci.ymljob passes on this head merged intomain, with 1408 tests, 1414 with #477's harness fix merged too, and the iOS dev build on Xcode 27.1, whereci.ymlpins 26.6. The changed native tests also pass with every sheet compiledinlineVariables: falseand after a JSON round trip, as Metro hands a sheet to a device. Theprefers-color-schemearm takes 9.8 ns per evaluation againstmain's 9.8 with a scheme held, and 11.1 against 8.9 where the held value names none, whichmainmatches to neither scheme, past the 1.2 ns betweenmain's own runs (jest, Node 26).Device evidence, before and after: #415 (comment)
Related
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 in one function and resets the reported and the requested scheme as this PR does, since the publicset()now asksAppearance.requestedColorSchemebesidecolorScheme.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.