feat(native): interop react-native-gesture-handler's Pressable and button family - #416
Open
YevheniiKotyrlo wants to merge 6 commits into
Open
feat(native): interop react-native-gesture-handler's Pressable and button family#416YevheniiKotyrlo wants to merge 6 commits into
YevheniiKotyrlo wants to merge 6 commits into
Conversation
Gesture Handler renders `GestureHandlerButton`, a codegen'd native component, so the Metro resolver's `react-native` rewrite never reaches it and `className` falls through the prop spread onto a view that declares no such prop. The class string is computed correctly and reaches no pixel. Adds a `components/react-native-gesture-handler` wrapper on the same shape as the existing `react-native-safe-area-context` one: `export *` plus a `useCssElement` re-declaration per affected component, a `nativeResolver` branch, and the exports entry whose `react-native` condition points at the native source. `RawButtonProps` gains `className` in types.d.ts. Every other styled component here reaches it through React Native's own props, but the button family extends neither `ViewProps` nor `TouchableWithoutFeedbackProps`.
The exclusion register stays — which components are deliberately not re-declared, and why, is the part a reader would otherwise undo — but as a terse list rather than four paragraphs of argument.
`Pressable` declares `style` as either styles or `(state) => styles` and picks
between them with `typeof style === "function"`. The className merge turned that
into an array, so the check answered "object", the callback never ran, and the
raw function reached the view — every pressed-state style silently dropped.
Measured on `<Pressable className="text-red" style={({pressed}) => …} />`:
`[{color:"#f00"}, [Function style]]` before, `[{color:"#f00"}, {opacity:1}]` after.
`style` merges at four sites inside `deepMergeConfig` — the inline pass and the
important pass of the length-1 `["style"]` branch, the length-1 array-target
block that overwrites the first of those, and the string-target path — so the
guard goes on all four rather than on the one the first reproduction hit.
`nativeResolver` takes its resolver as its first argument, so a recording `CustomResolver` drives every branch with no Metro: the three module rewrites, the `react-native/Libraries/*` lookup against `allowedModules`, and each of the four cases that must pass through untouched. `src/__tests__/metro/` is the first test directory for this module. That coverage turned up `isFromThisModule` resolving one level too high whenever the `source` export condition wins, because it counted a fixed number of levels up from `__dirname` and the built layout is a directory deeper than the source one. A missed exemption is a resolution cycle — this package's own components sent back through the wrapper that imports them — so the anchor is now the segment that names the layout.
`PureNativeButton` is a sixth member of the button family — the same codegen'd
`RNGestureHandlerButton`, exported directly — and was neither re-declared nor
excluded. Measured before: `{"type":"RNGestureHandlerButton","props":{"className":"pnb"}}`.
`DrawerLayoutAndroid` was excluded on the grounds that `components/index.cts`
re-exports it from react-native, but gesture-handler wraps it in
`createNativeWrapper` rather than re-exporting it, so the styled twin the
exclusion assumed does not exist and the class was dropped. Both are re-declared;
both forward `style`.
Two register entries were wrong about mechanism. The touchables were said to be
reached by the rewrite on non-Android platforms; they are not — `index.cts` has
no styled `TouchableNativeFeedback`, so the rewrite hands back react-native's own.
`DrawerLayout` and `Swipeable` were called a design decision; both are
`@deprecated`, which is the same ground the touchables stand on and is checkable.
`RefreshControl` now says plainly that the class is dropped.
The census the cases are generated from is derived from the module — a member is
re-declared iff its export is no longer the one `export *` provided — so a
seventh is covered the moment it lands, and an accounting test requires every
remaining export to name its reason. That pairing is what makes the omission
this commit fixes impossible to repeat.
Test C asserted an absence over an empty set: raw `RawButton` renders no `style`
prop at all, so it passed with a misspelled class or a component rendering
nothing. It now pins that the same declaration reaches the re-declared twin
first. `flattenStyles` flattened one array level and so could not see the nested
shape the Pressable merge produces.
`react-native-gesture-handler-rewrite.test.tsx` renders the five excluded
className-aware re-exports under the `react-native` rewrite, which is what the
exclusion actually claims; the object-identity assertions beside it are what
`export *` guarantees by construction.
Neither fix on this branch had a compiler-plane test, and for the interop the honest statement there is that the plane carries nothing: `compile` takes a CSS string and no component, so no artifact it emits can know who consumes it. `compiler/react-native-gesture-handler.test.tsx` measures that rather than asserting it — one compiled declaration is asserted byte-for-byte, then driven into a react-native primitive and every re-declared gesture-handler member, with each unwrapped twin beside it leaving the same bytes unresolved. Pointing RectButton's mapping at a prop that is not `style` reddens five render cases and leaves both compile assertions green; that insensitivity is the proof. `compiler/important.test.tsx` is the other fix's compiler half, and `!important` had no compiler coverage at all. The marker lands in the rule's specificity array at `Specificity.Important`, read through the exported census rather than the literal index, and it is what selects which of `deepMergeConfig`'s two passes a class takes — visible as the operand order inverting between them. A callback `style` has to survive whichever pass the marker selects, so the two are tested together. The exclusion register's claims are now measurements. The five members said to be reached by the `react-native` rewrite are shown LEAKING the raw class string without it, which is what makes the rewrite load-bearing rather than a comment. `TouchableNativeFeedback` and `DrawerLayoutAndroid` are shown resolving to react-native's own object under that rewrite, beside five names that resolve to a styled twin — so "the rewrite reaches it" and "the class survives" are separated by object identity. Gesture Handler's `DrawerLayoutAndroid` is pinned as its own `createNativeWrapper` component rather than a re-export. The census the drop invariant is generated from is derived from the export surface — every member that renders and is not re-declared — rather than from the union of the reason buckets. The two differ exactly when a member has been missed, so an unhandled component is rendered and held to the invariant on the commit that introduces it: removing the `PureNativeButton` re-declaration now fails on the leak itself, not only on the accounting. `deepMergeConfig` guards a callback at four sites and two of them were unobserved. The string-target branch never enters the array handling, so nothing the `["style"]` cases assert reaches it; it now has a test, and reverting that guard reddens it alone. Reverting the inline `["style"]` guard still reddens nothing: the length-1 array block recomputes the same key from the same operands a few lines later, so that site executes and its result is discarded. It is left in place as the symmetric form, but it is not load-bearing and no test can make it so. The three suites shared a census and two tree walks by copy; `_gesture-handler.ts` holds one of each. It imports neither `react-native` nor gesture-handler at module scope, because the rewrite suite mocks the former and a module-scope import would resolve gesture-handler through the mock and change what the census means.
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
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
react-native-gesture-handler'sPressableand button family renderGestureHandlerButton, a codegen'd native component. Thereact-nativeimport rewrite never reaches them, soclassNamefalls through their prop spread onto a view that declares no such prop — it reaches no pixel, and nothing warns. Measured on device: a gesture-handlerPressablecarryingflex flex-row items-center gap-2 rounded-md px-3 py-2rendered 22×114 px against a plainViewcontrol's 132×101, stacked vertically becauseflex-rownever applied.The type side has the same hole on the buttons.
RawButtonPropsextendsNativeViewGestureHandlerPropsandAccessibilityProps, neverViewProps, soBaseButtonProps,RectButtonPropsandBorderlessButtonPropsnever inherit theclassNamethis library adds. (RNGH's ownPressabledoes reach it — its props areOmit<ViewProps, 'children' | 'style' | 'hitSlop'>, which keepsclassName.)Fix
A styled
react-native-gesture-handlerentry, resolved the way the existingreact-native-safe-area-contextone is, mappingclassNameonto thestyleeach component already forwards. Plus aRawButtonPropsaugmentation intypes.d.tsfor the button family.Seven components are re-declared:
Pressable,RawButton,BaseButton,RectButton,BorderlessButton,PureNativeButtonandDrawerLayoutAndroid.Which components are not re-declared, and why, is the part worth reviewing. Every line of it is now a test rather than a comment:
ScrollView,Switch,TextInput,FlatList,Text—createNativeWrapperforwards unclaimed props to a react-native primitive, andTextrenders one directly, so thereact-nativerewrite already reaches these. Wrapping them would style the gesture handler rather than the view.DrawerLayout,Swipeable—classNameis dropped on all six. Gesture Handler marks every one@deprecatedin favour ofPressableand the Reanimated twins, and this repo lints@typescript-eslint/no-deprecatedaterror, so wrapping them would need a suppression in your own source for a surface scheduled for deletion.RefreshControl—classNameis dropped. React Native's jest mock renders<RCTRefreshControl />with no props at all, so no test at this tier could observe a fix, andstyleon a RefreshControl drives nothing on either platform.ReanimatedDrawerLayout,ReanimatedSwipeable— out of reach: they ship as their own entry points rather than from the index, and the resolver branch matches an exactreact-native-gesture-handler.Two things a later pass found, and what now stops them recurring
PureNativeButtonis a sixth member of the button family — the same codegen'dRNGestureHandlerButton, exported directly — and the first version of this PR neither re-declared nor excluded it. It rendered{"type":"RNGestureHandlerButton","props":{"className":"pnb"}}: the raw class string on a native view, which is worse than dropping it.Two exclusions did not do what they claimed.
TouchableNativeFeedbackwas excluded on the grounds that it "re-exports React Native's, which the rewrite already reaches". The rewrite does reach the specifier — andcomponents/index.ctshands backrequire("react-native").TouchableNativeFeedbackuntouched, with no styled twin, so the class is dropped on every platform rather than only on Android.DrawerLayoutAndroidwas excluded as re-exported straight fromreact-native; gesture-handler wraps it increateNativeWrapperinstead, so the styled twin that exclusion assumed does not exist in either direction, and the class was simply dropped. It is now re-declared.Both are structural, not clerical, so the guards are too:
export *provided, and the excluded set is every remaining export that renders. The two are computed, not listed, so an eighth component landing upstream joins the generated cases on the commit that introduces it.PureNativeButtonre-declaration now fails onclassNamesurviving onto the rendered element, which is the defect itself — not on a name being absent from a list.TouchableNativeFeedbackandDrawerLayoutAndroidare shown resolving to react-native's own object under a real rewrite, besideView,Text,ScrollView,TouchableOpacityandTouchableHighlight, which resolve to a styled twin. "The rewrite reaches it" and "the class survives" are separated by object identity rather than by prose.A core defect this also fixes: a callback
styleprop stopped being a callbackdeepMergeConfiginsrc/native/styles/index.tsmerged astylevalue as data. A function fails both object tests, so it fell into the array branch and producedstyle: [computed, theCallbackItself]— an array containing the un-invoked function.Pressablepicks its branch withtypeof style === "function"; wrapped in an array the answer is"object", so the callback never ran and the raw function reached the native view. Every pressed-state style silently gone, no warning.Measured on
<Pressable className="text-red" style={({pressed}) => …} />:[{color:"#f00"}, [Function style]]before,[{color:"#f00"}, {opacity:1}]after.No gesture-handler code is on that path.
src/components/Pressable.tsxcarries the identical{ className: "style" }mapping and is the twin thereact-nativerewrite installs across the whole graph, so every<Pressable className=… style={({pressed}) => …}>in any app was affected, as was any userstyled()mapping whose target prop holds a callback. Its regression tests importreact-native-css/components/Pressableand no gesture-handler at all. Happy to split it into its own PR if you would rather — say the word and I will.stylemerges at four sites insidedeepMergeConfigand the guard is on all four. Two notes from measuring them:["style"]cases assert reaches it. It has its own test; reverting that one guard reddens it alone.["style"]guard executes but its result is discarded: the length-1 array block a few lines down recomputes the same key from the same operands. Reverting it reddens nothing, and no test can make it. I left it as the symmetric form — say if you would rather it went.Tests
Both fixes are tested on both planes.
Compiler plane.
src/__tests__/compiler/react-native-gesture-handler.test.tsxsays what that plane carries for the interop, which is nothing — and measures it rather than asserting it. One declaration is asserted byte-for-byte, then driven into a react-native primitive and into every re-declared gesture-handler member, with each unwrapped twin beside it leaving the same bytes unresolved. PointingRectButton's mapping at a prop that is notstylereddens five render cases and leaves both compile assertions green; that insensitivity is the proof the fix does not belong on this plane. It also contrasts the emitted declaration against@nativeMapping's, so a reader looking for a compiler feature behind the interop can see there is none.src/__tests__/compiler/important.test.tsxis the second fix's compiler half, and!importanthad no compiler coverage in the repo at all. The marker lands in the rule's specificity array atSpecificity.Important— read through the exported census rather than the literal index — and it is what selects which ofdeepMergeConfig's two passes a class takes, visible as the operand order inverting between them. A callback has to survive whichever pass the marker selects, so the two are asserted together.Native plane. Per re-declared component: the fix (
classNameresolves into the rendered style), the invariant (classNamenever reaches a rendered element as a prop), the trigger (the unwrapped component drops it, so the bug is pinned and cannot silently stop being a bug), coexistence with an inlinestyle, and byte-identical output when noclassNameis given — that last one because the resolver routes every gesture-handler import in the graph through this wrapper, react-navigation and react-native-screens included. Each registers a width no other test uses.react-native-gesture-handler-rewrite.test.tsxrenders the five rewrite-dependent members under a realreact-native→react-native-css/componentsmock. Its sibling shows the same five leaking the raw class string without that mock, which is what makes the rewrite load-bearing rather than a comment.Every generated census carries a non-empty assertion, so a narrowed export surface cannot quietly produce zero cases.
Gates
Measured at the head of this branch, on Windows:
yarn typecheckyarn lintyarn buildyarn testTest Suites: 2 failed, 4 skipped, 58 passed, 60 of 64 total/Tests: 3 failed, 21 skipped, 1170 passed, 1194 totalThe 3 failures are Windows-only and pre-existing —
react-native › plugin › 7,react-native-web › plugin › 6andreact-native-web › plugin › 17, path-rewrite cases expectingrequire("react-native-css/components/View")and gettingrequire("../View"). They are red on pristinemainon this machine and are the exact shape #390 addresses. Your CI is green on them, so please read "3 pre-existing failures" as machine-local rather than as a claim about the repo. Run twice before reading any total on Windows — a cold cache can drop a whole suite, which subtracts its count with zero failures to show for it.react-native-gesture-handler@2.28.0is added as an exact-pinned devDependency — the version Expo SDK 54 bundles, mirroring howreact-native-safe-area-context: 5.6.1is pinned against that SDK's~5.6.0.Open questions
src/metro/resolver.tschanges resolution beyond gesture-handler, twice. The new branch rewrites every exactreact-native-gesture-handlerimport in the graph — defended by the byte-identical-output test, and narrow enough that subpath imports are untouched. Separately, the own-package exemption is rewritten from a fixed../../../depth to a layout-anchored package root: under thereact-nativeexport condition__dirnameis<pkg>/src/metro, and the old derivation resolved tonode_modules/src, so the exemption missed and this package's own modules were fed back through the wrappers that import them. That is a real fix on a path shared by thereact-native, safe-area andreact-native/Libraries/*rewrites.DrawerLayoutandSwipeableare excluded as@deprecated. If you would rather they were styled, the mapping is the same{ className: "style" }and the exclusion is one register entry to move — thoughDrawerLayoutandSwipeablewould still need a decision about which ofcontainerStyle/childrenContainerStyle/drawerContainerStyleclassNametargets, and whether the others get their own*ClassNameprops the wayScrollViewhascontentContainerClassName.types.d.ts.react-native-gesture-handleris a devDependency here, not a peer, and there is nopeerDependenciesMetablock — mirroring howreact-native-safe-area-contextis carried. So the resolver unconditionally rewrites a package that is neither declared nor optional, andtypes.d.tsaugments a module a consumer may not have installed. Measured not to break such a consumer (adeclare modulenaming an absent package compiles clean underskipLibCheck: false), but it is a new pattern and you may prefer an opt-in./types/*entry.src/__tests__/metro/is a new test directory for a module that had none, andsrc/__tests__/_gesture-handler.tsis a shared helper on the_-prefix conventionvendor/tailwind/_tailwind.tsxalready uses.Interaction with #418. This PR adds a
react-native-gesture-handlermodule augmentation totypes.d.ts; #418 rewrites that same file to declare every prop| undefinedforexactOptionalPropertyTypes. The two overlap textually and whichever lands second will need a trivial rebase. The newRawButtonPropsblock here already declares both of its props| undefined, so it is consistent with #418's rule either way — but if #416 lands first, #418's sweep should be re-run so the file has no exception, and if #418 lands first, nothing here changes.