fix: Remove reference to Touchable in package types#592
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughUpdated React Native type augmentations in ChangesType Declaration Update
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR removes the
Confidence Score: 5/5Safe to merge — this is a minimal type-only refactor with no functional changes. The change is confined to a single .d.ts file and removes three deprecated imports while aliasing a single replacement. The PR description correctly identifies that onTouchStart and related members are already on ViewProps, making the old Touchable extension redundant. All existing props are preserved, and the new base type structurally encompasses everything the old individual bases provided. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix: Remove reference to Touchable in pa..." | Re-trigger Greptile |
f145833 to
341c40b
Compare
Summary: **Motivation** The public type for `Touchable` has zero overlap with its runtime value. This is an internal implementation object which is undocumented. https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts?lines=18-24 https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.js?lines=966-986 Replaces D110483989 / react#57419. **Impact** This is a **breaking change** with few open source consumers. The `onTouchStart` etc prop typings are part of `ViewProps` today, which is a suitable replacement. Across projects I've been testing for the Strict TypeScript API rollout, the only load bearing consumer was Uniwind — where I've sent a PR to address this. - uni-stack/uniwind#592. Changelog: [General][Breaking] - The `Touchable` root export (undocumented) is removed. If you are extending `Touchable` as a type, please use `ViewProps` instead. Reviewed By: rubennorte Differential Revision: D110488985
Summary: **Motivation** The public type for `Touchable` has zero overlap with its runtime value. This is an internal implementation object which is undocumented. https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts?lines=18-24 https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.js?lines=966-986 Replaces D110483989 / react#57419. **Impact** This is a **breaking change** with few open source consumers. The `onTouchStart` etc prop typings are part of `ViewProps` today, which is a suitable replacement. Across projects I've been testing for the Strict TypeScript API rollout, the only load bearing consumer was Uniwind — where I've sent a PR to address this. - uni-stack/uniwind#592. Changelog: [General][Breaking] - The `Touchable` root export (undocumented) is removed. If you are extending `Touchable` as a type, please use `ViewProps` instead. Reviewed By: rubennorte Differential Revision: D110488985
Summary: Pull Request resolved: #57420 **Motivation** The public type for `Touchable` has zero overlap with its runtime value. This is an internal implementation object which is undocumented. https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts?lines=18-24 https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.js?lines=966-986 Replaces D110483989 / #57419. **Impact** This is a **breaking change** with few open source consumers. The `onTouchStart` etc prop typings are part of `ViewProps` today, which is a suitable replacement. Across projects I've been testing for the Strict TypeScript API rollout, the only load bearing consumer was Uniwind — where I've sent a PR to address this. - uni-stack/uniwind#592. Changelog: [General][Breaking] - The `Touchable` root export (undocumented) is removed. If you are extending `Touchable` as a type, please use `ViewProps` instead. Reviewed By: rubennorte Differential Revision: D110488985 fbshipit-source-id: 394e3996bb45482723a4b7b18c1cf63f49cb49c5
|
Hey, thanks for PR 🤝 |
Summary: NOTE: Resubmission of D110488985. **Motivation** The public type for `Touchable` has zero overlap with its runtime value. This is an internal implementation object which is undocumented. https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts?lines=18-24 https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.js?lines=966-986 Replaces D110483989 / react#57419. **Impact** This is a **breaking change** with few open source consumers. The `onTouchStart` etc prop typings are part of `ViewProps` today, which is a suitable replacement. Across projects I've been testing for the Strict TypeScript API rollout, the only load bearing consumer was Uniwind — where I've sent a PR to address this. - uni-stack/uniwind#592. Changelog: [General][Breaking] - The `Touchable` root export (undocumented) is removed. If you are extending `Touchable` as a type, please use `ViewProps` instead. Reviewed By: rubennorte Differential Revision: D111022549
Summary: NOTE: Resubmission of D110488985. **Motivation** The public type for `Touchable` has zero overlap with its runtime value. This is an internal implementation object which is undocumented. https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts?lines=18-24 https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.js?lines=966-986 Replaces D110483989 / react#57419. **Impact** This is a **breaking change** with few open source consumers. The `onTouchStart` etc prop typings are part of `ViewProps` today, which is a suitable replacement. Across projects I've been testing for the Strict TypeScript API rollout, the only load bearing consumer was Uniwind — where I've sent a PR to address this. - uni-stack/uniwind#592. Changelog: [General][Breaking] - The `Touchable` root export (undocumented) is removed. If you are extending `Touchable` as a type, please use `ViewProps` instead. Reviewed By: rubennorte Differential Revision: D111022549
Summary: Pull Request resolved: #57476 NOTE: Resubmission of D110488985. **Motivation** The public type for `Touchable` has zero overlap with its runtime value. This is an internal implementation object which is undocumented. https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts?lines=18-24 https://www.internalfb.com/code/fbsource/[86267213ee84]/xplat/js/react-native-github/packages/react-native/Libraries/Components/Touchable/Touchable.js?lines=966-986 Replaces D110483989 / #57419. **Impact** This is a **breaking change** with few open source consumers. The `onTouchStart` etc prop typings are part of `ViewProps` today, which is a suitable replacement. Across projects I've been testing for the Strict TypeScript API rollout, the only load bearing consumer was Uniwind — where I've sent a PR to address this. - uni-stack/uniwind#592. Changelog: [General][Breaking] - The `Touchable` root export (undocumented) is removed. If you are extending `Touchable` as a type, please use `ViewProps` instead. Reviewed By: rubennorte Differential Revision: D111022549 fbshipit-source-id: bf29ae826d34fb04889b9f56de0f11904fce3f53
Motivation
Align Uniwind with modern/real types from the
react-nativepackage. This is a load bearing breakage for users under React Native's Strict TypeScript API and we are likely removing this export in React Native 0.87.Problem
We found a load bearing call site in Uniwind where the
Touchabletype is imported fromreact-nativeand used for theScrollViewPropstype augmentation.uniwind/packages/uniwind/types.d.ts
Lines 1 to 7 in 6b498f2
uniwind/packages/uniwind/types.d.ts
Line 40 in 6b498f2
There are two issues with this:
Touchabletype shouldn't really exist in core (see below).onTouchStartetc are already present onViewProps.Fix
Simplify
types.d.tsto factor outTouchable. No type shape / functional changes.Mini-dive: This was a nonsense type(!)
Touchabletoday is a manually maintained TypeScript definition shadowing a runtime object that has no overlap.Current (manual) TS definition:
https://github.com/react/react-native/blob/a7959ddc983855cc40390d637aadbfb213e7c3a7/packages/react-native/Libraries/Components/Touchable/Touchable.d.ts#L18-L24
vs the real object with two props — an internal implementation object with
MixinandrenderDebugView:https://github.com/react/react-native/blob/a7959ddc983855cc40390d637aadbfb213e7c3a7/packages/react-native/Libraries/Components/Touchable/Touchable.js#L966-L971
We're actually going to pursue removing this, which may land in RN 0.87.
Summary by CodeRabbit
Summary by CodeRabbit