fix: compile on every react-native in the peer range, not just 0.81 - #429
fix: compile on every react-native in the peer range, not just 0.81#429YevheniiKotyrlo wants to merge 4 commits into
Conversation
jest-expo 56 declares @react-native/jest-preset as a non-optional peer and resolves setupFiles and testEnvironment through it. Nothing provided it, so the suite quietly ran on the copy react-native ships in-tree. react-native 0.86 deleted that copy. Its jest-preset.js is now a shim that throws unless @react-native/jest-preset is installed, which takes the suite down in jest.config.cjs before a single test loads. Declaring the peer keeps the config loadable across the whole peer range. Verified on react-native 0.81.4 that the preset changes nothing: 1048 passed, 3 failed, 21 skipped, 1072 total both with and without it, where the 3 failures are pre-existing Windows path mismatches in the babel-plugin-tester suites.
react-native's ColorSchemeName is not stable across this package's `react-native: >=0.81` peer range. Up to 0.85 it is "light" | "dark" | null | undefined; from 0.86 it is "light" | "dark" | "unspecified", and Appearance.setColorScheme rejects null. Six of the seven type errors the package produces on 0.86 come from that one inversion, and while the type is re-exported from react-native no single source can satisfy both ends of the range. ColorSchemeName is now the package's own union of every spelling, so colorScheme is one API across the range whichever version is installed. The one call the two versions declare incompatibly, the "follow the system" write, goes through setAppearanceColorScheme: "light" and "dark" keep the shared signature, and everything else is sent as "unspecified". That reaches the native module identically on both floors, because 0.85 and earlier map null to "unspecified" themselves before handing it down. The new suite pins both planes. The type aliases are read off the installed react-native rather than restating either spelling, so typecheck fails at whichever end of the range the checkout is on if coverage ever lapses; the runtime cases assert what the native module actually receives.
ScrollViewProps.scrollViewRef is a RefObject<ScrollView>, and ScrollView holds ScrollViewProps again, so the prop graph DotNotation walks is cyclic. Bounded at MaxDepth 10 that single prop contributes 4,654 paths on react-native 0.81, and on 0.86, where the host instance surface is wider, building the union crosses TypeScript's 100,000 member limit: StyledConfiguration<typeof FlatList> fails with TS2590 and takes src/components/FlatList.tsx with it. None of those paths can ever be a mapping target. They run through an object that does not exist until render. Treating a class component instance as a leaf cuts the cycle at its source, rather than lowering MaxDepth, which would only move the threshold to the next prop type that grows. Measured on react-native 0.81.4, where both the old and new definitions resolve: ViewProps is unchanged at 733 paths, ScrollViewProps drops from 7,482 to 2,094 and FlatListProps from 9,204 to 3,816. Every removed path traverses a ref, and nothing is added.
Measured `yarn typecheck` on upstream/main against every minor in the peer range, one install each: 0.81.4 gives 0 errors, 0.82.0 gives 6, and 0.83.0 through 0.86.0 give 7. So the color scheme break starts at 0.82, where ColorSchemeName became "light" | "dark" | "unspecified" and setColorScheme stopped accepting null, and the TS2590 break starts at 0.83, where the wider host instance surface pushes the cyclic ref enumeration over TypeScript's union limit. 0.81 is the only minor in a `>=0.81` range that compiles. The runtime half moves with it: 0.82 and later pass the argument straight to the native module, so only 0.81 maps null to "unspecified" on the way in. That makes "unspecified" the value that is right everywhere, which is what the code already sends; only the comments named the wrong boundary.
YevheniiKotyrlo
changed the title
|
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 — |
1 participant
peerDependenciessaysreact-native: ">=0.81". 0.81 is the only minor in that range the package compiles on. Measured withyarn typecheckatf70c402, one install per version:react-nativeTwo unrelated causes, with different start points, plus a jest bootstrap failure that makes the suite unrunnable on 0.86. This PR makes all six compile and keeps 0.81 exactly where it is.
The seven on 0.86:
1.
ColorSchemeNameinverts at 0.82 (6 errors, 0.82 and later)react-native's own type changes meaning inside the declared range:ColorSchemeNameAppearance.setColorSchemeacceptsnullat runtime"light" | "dark" | null | undefinedColorSchemeName | null | undefined"unspecified""light" | "dark" | "unspecified"ColorSchemeName—nullrejectedruntime.types.tsre-exported that type asColorScheme's parameter and return, so the package's public API changed shape with the installedreact-native, and the implementation could not satisfy both ends. The four errors reporting a bare genericValueare cascades of the failedobservable<ColorSchemeName>(...)call atreactivity.ts:220.The two
setColorSchemeparameter types overlap only on"light" | "dark", so no literal naming "follow the system" type-checks on both. The runtime does not split the same way:"unspecified"is what the native module receives on every version, because 0.81 mapsnullto it on the way in.ColorSchemeNameis now the package's own union of every spelling,"light" | "dark" | "unspecified" | null | undefined. Both directions widen, so nothing a caller passes today stops working, and"unspecified"becomes writable on 0.81 too. The one incompatible call goes throughsetAppearanceColorScheme:"light"and"dark"keep the shared declaration, everything else is written as"unspecified"through a locally restated signature — one place, reason inline, covering only the value the versions disagree about.2.
DotNotationwalks into component instances (TS2590, 0.83 and later)Different cause, so worth reading separately.
ScrollViewProps.scrollViewRefis aRefObject<ScrollView>, andScrollViewcarriesScrollViewPropsagain — the prop graphDotNotationenumerates is cyclic. Bounded atMaxDepth 10, that one prop contributes 4,654 paths on 0.81. From 0.83 the host instance surface is wide enough that building the union crosses TypeScript's 100,000-member limit, soStyledConfiguration<typeof FlatList>fails and takessrc/components/FlatList.tsxwith it.FlatList.d.tsis unchanged across all of these versions; the growth is underneath it.None of those paths can ever be a mapping target — they run through an object that does not exist until render. Treating a class component instance as a leaf removes the cycle at its source. Lowering
MaxDepthwould only move the threshold to whichever prop type grows next.Path-set diff on 0.81, where the old and new definitions both resolve:
ViewPropsScrollViewPropsFlatListProps<unknown>Pick<ScrollViewProps, "scrollViewRef">Every removed path traverses a ref (
.current.). Nothing is added.3. The suite cannot bootstrap on 0.86
Independent of the types, and a prerequisite for testing any of this.
jest-expo@56declares@react-native/jest-presetas a non-optional peer and resolvessetupFilesandtestEnvironmentthrough it. Nothing provided it, so the suite ran on the copyreact-nativeshipped in-tree. 0.86 deleted that copy; itsjest-preset.jsis now a shim that throws, and.config/jest.config.cjsdies before a single test loads:The package has no 0.81.x release — 0.85.0 is the first — so it cannot be pinned to this repo's
react-native.^0.85.0matches jest-expo's own declared range and works at both ends. On 0.81.4 it is a no-op: the suite is bit-identical with and without it.Verification
Typecheck, every minor in the range, one install each, root and
example/manifests moved together:upstream/mainLint, build and the full suite at both ends of the range:
react-native@0.81.4react-native@0.86.0yarn lintyarn buildyarn test3 failed, 21 skipped, 1055 passed, 1079 total3 failed, 21 skipped, 1055 passed, 1079 totalIdentical, with
numRuntimeErrorTestSuites: 0on each — every suite loaded. The 3 failures are pre-existing onmainbefore this branch:babel-plugin-testerpath mismatches over an unrewritten relativerequire("../View"), reproducing on Windows only. Every figure above is the second of two consecutive runs, since a cold first run in a fresh tree can drop suites and silently subtract their whole count.Running the suite on 0.86 also needs
@react-native/babel-presetat 0.86 —babel-preset-expo@54pins 0.81.4, whose codegen plugin cannot parse 0.86's native component specs (Unable to determine event arguments for "onChange", 21 suites down). That is an Expo SDK pairing issue rather than anything in this package, so it is not in the diff; I pinned it throughresolutionslocally for the measurement only.Tests
Both fixes are type-surface, so each carries a compile-plane assertion, and the color scheme one carries a runtime assertion as well.
src/__tests__/native/color-scheme.test.tsxColorSchemeNamecoversreact-native's own union,Appearance.getColorScheme()'s return, and the change-listener payload. They are read off the installedreact-nativerather than restating either spelling, soyarn typecheckfails at whichever version the checkout is on the moment coverage lapses.NativeAppearanceis mocked so writes are observable — it isTurboModuleRegistry.get('Appearance'), null under jest on every version, which is whysetColorSchemewas previously unobservable in tests. Five cases assert what the native module actually receives; a sixth asserts@media (prefers-color-scheme: dark)engages on"dark"and releases on"unspecified".src/__tests__/native/components.test.tsxscrollViewRefandscrollViewRef.currentare reachable targets, and a@ts-expect-errorpins thatscrollViewRef.current.props.styleis not. That directive is what makes the guard work on 0.81 and 0.82 too, where TS2590 never fires — reverting the fix there fails withTS2578: Unused '@ts-expect-error' directive.dot-notation.types.tscompiles toexport {}(72 bytes), so it has no runtime surface.FlatList's mapping behaviour is already covered byclassName-with-style.test.tsx, which passes at both ends.Each fix was mutation-proved by reverting it and watching the guard go red:
TS2578unused@ts-expect-errorTS2578+ the originalFlatList.tsx(12,16) TS2590ColorSchemeNameTS2345oncolorScheme.set("unspecified")web/api.tsx TS2322setAppearanceColorSchemesends the raw valuenullitself, so this version cannot see itnull/undefined@react-native/jest-presetdevDependencyOut of scope, noticed while here
react-native-web@0.21'sAppearanceexportsgetColorSchemeandaddChangeListenerbut nosetColorScheme, socolorScheme.set()on web throwsTypeErrorat runtime today. Unrelated to this, and unchanged by this PR.