fix(touchable-ripple): support function children and style on native - #5046
fix(touchable-ripple): support function children and style on native#5046giaBaoJS wants to merge 1 commit into
Conversation
The native implementation typed `children` as `React.ReactNode` and `style` as `StyleProp<ViewStyle>`, while the web implementation and the documentation both allow them to be render functions receiving the pressable state. As a result, passing a function to `children` threw "React.Children.only expected to receive a single React element child." and passing a function to `style` was silently dropped. Both branches of the component now resolve the render functions with the state provided by `Pressable`, matching the web implementation. Static children and styles are still forwarded unchanged, so `Pressable` keeps skipping pressed state updates for the components that do not need them. `BottomNavigationBar` forwards `TouchableRipple`'s `style` prop to a plain `Pressable` on its fallback path, so it now uses the internal `Pressable` wrapper which shares the same state callback type.
|
Thanks for the review @ziarno. I think this one is stuck on something outside the diff. The only check that has ever run on it is It is not specific to this PR. Every recent outside-contributor PR I looked at shows the same single check, while #5062 ran the full set (Unit tests, Lint, Typecheck, Build package, Build docs, Build example, CodeQL). So it looks like the workflow-approval gate for outside contributors rather than anything in the branch. Could someone with write access kick off the workflow run? The same applies to my other open PRs here: #5047, #5048, #5064, #5065, #5066. Branch is 3 commits behind |
Motivation
TouchableRippledocumentschildrenandstyleas accepting a render function that receives the pressable state, andTouchableRipple.tsx(web) implements exactly that.TouchableRipple.native.tsxdoes not, so the two platforms disagree:src/components/TouchableRipple/TouchableRipple.native.tsx(before this PR)vs.
src/components/TouchableRipple/TouchableRipple.tsx:66-72Measured on
maintoday:childrenthrowsReact.Children.only expected to receive a single React element child., from the bareReact.Children.only(children)calls atTouchableRipple.native.tsx:98(ripple branch) and:121(highlight-fallback branch);styleis passed straight into a plain array, so it is silently dropped — the renderedprops.styleis[false, null], with no error at all.The divergence dates back to #3909, which reworked the web file only.
What this PR changes
TouchableRipple.native.tsxnow mirrors the web contract on both of its branches. BothReact.Children.onlycall sites are handled, since both are reachable —:98on Android >= Lollipop,:121everywhere else — and a fix to only one of them would just move the bug to the other platform.One deliberate difference from a literal copy of the web file: the render-function form is used only when the corresponding prop is actually a function.
PressablederivesshouldUpdatePressedfromtypeof children === 'function' || typeof style === 'function'(Pressable.js:224-225), so unconditionally passing a functionstylewould make everyButton,Card,List.Item,Chip,DataTable.Row,Menu.Item,IconButton,Checkbox,RadioButton,Drawer.ItemandSegmentedButtonsitem start re-rendering on every press-in/press-out, even though none of them use the state. Keeping the static form static makes this fix inert for the common path. The web file can pass a function unconditionally because it always needsstate.hovered.React.Children.onlyis kept on the resolved children, matching the web file's behaviour exactly. If #5018 lands, both call sites can be dropped together in that refactor.The one line outside
TouchableRipple:BottomNavigationBarforwardsTouchableRipple'sstyleprop to a barereact-nativePressableon its non-ripple path, and RN'sPressableStateCallbackTypeis{ pressed }while the wrapper's is{ pressed, hovered, focused }. It now imports the samePressablewrapperTouchableRippleuses — which isPressableNative as any, so this is a types-only change with no runtime effect — otherwiseyarn typecheckfails there.Related issue
Fixes #4873
Related: this looks like a missed entry on the v6
React.Childrenchecklist in #4989 — #5018 touches 30 files and none of them isTouchableRipple.There is also #4896 from @skainguyen1412, which addresses the same report, but it targets the
5.0branch, somainis still affected. I'm happy to close this one if that PR gets forward-ported instead; this PR additionally covers the ripple-supported branch (:98), the functionstylecase, and thePressablepressed-state-tracking concern above.Test plan
yarn lint,yarn typecheckandyarn testall pass.src/components/__tests__/TouchableRipple.test.tsxgains 8 tests, run twice viadescribe.each— once withTouchableRipple.supported = false(highlight fallback) and once withtrue(native ripple) — so bothReact.Children.onlycall sites are covered:supports children as a render functionpasses the pressed state to the children render function(viatestOnly_pressed, so it proves the state is actually threaded, not just that a function is invoked)resolves style as a functionkeeps rendering element children with a style object— non-vacuity: the pre-existing path must be untouchedCounterfactual, with the source change reverted and the tests kept:
The three failures per branch are
React.Children.only expected to receive a single React element child.(thrown from:121and:98respectively) andtoHaveStylenot findingopacity: 0.5. With the fix applied, all 13 pass.Full suite before:
55 passed, 732 passed / 1 skipped, 169 snapshots.Full suite after:
55 passed, 740 passed / 1 skipped, 169 snapshots.Snapshot churn: none. All 169 snapshots pass unmodified, which is the check that matters here —
TouchableRippleunderpins most touchable components in the library, and the staticstylearray it renders is byte-identical to before.