From 574a2f9850a1268b8c40ba716cba922e64690f0b Mon Sep 17 00:00:00 2001 From: Idan Levi Date: Wed, 27 May 2026 10:09:57 +0300 Subject: [PATCH 1/2] fix(new-arch): align TurboModule openPicker signature across iOS, Android, and JS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Under React Native's new architecture, the TurboModule runtime enforces strict argument-count parity between the JS-side spec, the platform native module, and every JS caller. `react-native-date-picker` currently has three sources of truth for `openPicker` and they disagree: | Layer | Args | |-----------------------------------------------|---------------| | iOS native (`ios/RNDatePickerManager.mm`) | 3 (uses cbs) | | Android native newarch (`DatePickerModule`) | 1 | | Codegen spec (`src/fabric/NativeRNDatePicker.ts`) | 1 | | JS caller iOS (`src/modal.js`) | 3 | | JS caller Android (`src/modal.js`) | 1 | So one of the two platforms is always wrong: - With the spec at 1 arg, the iOS caller's 3-arg invocation is over by two (RN currently permits this, but it is undocumented behaviour and fails strict tooling). - If a consumer patches the spec to 3 args to match iOS (a common workaround once they hit `TurboModule method "openPicker" called with 1 arguments (expected argument count: 3)`), the Android JS caller starts crashing on every modal open. This PR makes all five layers agree on 3 args: - `src/fabric/NativeRNDatePicker.ts` — spec now declares `openPicker(props, onConfirm, onCancel)`. - `android/src/newarch/java/.../DatePickerModule.java` — accepts `Callback onConfirm, Callback onCancel` and ignores them; Android continues to deliver confirm/cancel via the existing `RCTDeviceEventEmitter` flow. - `src/modal.js` — Android branch now passes two no-op callbacks so the 3-arg signature is honoured on both platforms. iOS behaviour is unchanged at runtime (its native side already takes 3 args and uses them). Android behaviour is unchanged at runtime (the callbacks are unused; events still flow via the emitter listeners registered in `useModal`). Encountered downstream in a React Native 0.85 + new-arch banking app where opening any modal date picker (transaction filter date range, loan payment day) on Android crashed with the strict arg-count error. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../com/henninghall/date_picker/DatePickerModule.java | 9 ++++++++- src/DatePickerIOS.js | 8 ++++++-- src/fabric/NativeRNDatePicker.ts | 6 +++++- src/modal.js | 7 ++++++- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/android/src/newarch/java/com/henninghall/date_picker/DatePickerModule.java b/android/src/newarch/java/com/henninghall/date_picker/DatePickerModule.java index 0fce9a78..6676ba31 100644 --- a/android/src/newarch/java/com/henninghall/date_picker/DatePickerModule.java +++ b/android/src/newarch/java/com/henninghall/date_picker/DatePickerModule.java @@ -2,6 +2,7 @@ import androidx.annotation.NonNull; +import com.facebook.react.bridge.Callback; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReadableMap; @@ -28,7 +29,13 @@ public void removeListeners(double type) { } @Override - public void openPicker(ReadableMap props){ + public void openPicker(ReadableMap props, Callback onConfirm, Callback onCancel){ + // Android delivers confirm/cancel to JS via the RCTDeviceEventEmitter + // (see DatePickerModuleImpl), so the callbacks supplied here are + // unused. They exist only to keep the TurboModule signature + // consistent with the iOS native module, where iOS *does* use the + // callbacks. Without them the spec would have to diverge by + // platform, which the codegen tooling does not allow. module.openPicker(props); } diff --git a/src/DatePickerIOS.js b/src/DatePickerIOS.js index 987cbcac..268519bc 100644 --- a/src/DatePickerIOS.js +++ b/src/DatePickerIOS.js @@ -23,8 +23,12 @@ export const DatePickerIOS = (props) => { style: [styles.datePickerIOS, props.style], date: props.date ? props.date.toISOString() : undefined, locale: props.locale ? props.locale : undefined, - maximumDate: props.maximumDate ? props.maximumDate.toISOString() : undefined, - minimumDate: props.minimumDate ? props.minimumDate.toISOString() : undefined, + maximumDate: props.maximumDate + ? props.maximumDate.toISOString() + : undefined, + minimumDate: props.minimumDate + ? props.minimumDate.toISOString() + : undefined, theme: props.theme ? props.theme : 'auto', } diff --git a/src/fabric/NativeRNDatePicker.ts b/src/fabric/NativeRNDatePicker.ts index 1633d7a1..c3c187e8 100644 --- a/src/fabric/NativeRNDatePicker.ts +++ b/src/fabric/NativeRNDatePicker.ts @@ -5,7 +5,11 @@ import { Double, UnsafeObject } from 'react-native/Libraries/Types/CodegenTypes' export interface Spec extends TurboModule { readonly getConstants: () => {} closePicker(): void - openPicker(props: UnsafeObject): void + openPicker( + props: UnsafeObject, + onConfirm: (result: UnsafeObject) => void, + onCancel: () => void + ): void removeListeners(type: Double): void addListener(eventName: string): void } diff --git a/src/modal.js b/src/modal.js index fe02e33e..b1cc0ab5 100644 --- a/src/modal.js +++ b/src/modal.js @@ -76,8 +76,13 @@ export const useModal = ({ props, id }) => { useEffect(() => { if (shouldOpenModal(props, previousProps)) { closing.current = false + // The TurboModule spec for `openPicker` declares 3 args because iOS + // native uses the callbacks directly. Android routes confirm/cancel + // through the `NativeEventEmitter` registered below, so the + // callbacks here are no-ops — they exist only to satisfy the + // strict arg-count check in the new architecture. const params = Platform.select({ - android: [props], + android: [props, () => {}, () => {}], ios: [props, onConfirm, onCancel], }) if (!params) throw Error('Unsupported platform') From 2e72af5a1a8c76e7431090683c95c2c5437a8a7f Mon Sep 17 00:00:00 2001 From: Idan Levi Date: Wed, 27 May 2026 10:24:30 +0300 Subject: [PATCH 2/2] ci(lint): check out PR head, not master, under pull_request_target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pr.yml` is triggered by `pull_request_target`, which by default runs in the base-branch (master) context. Most workflows already override the checkout ref to the PR head: - uses: actions/checkout@v4 with: ref: ${{ github.event.pull_request.head.sha }} `lint.yml` was missing that override, so lint was always run against master's source rather than the PR head — meaning lint failures on the PR could not be fixed by the PR, and prettier issues already in master permanently broke every contributor's lint check. Mirror the existing pattern from build-android.yml / build-ios.yml / test-android-*.yml. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/lint.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index efae1e0d..701f3bbb 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -12,6 +12,14 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + # Required because pr.yml uses `pull_request_target`, which + # otherwise checks out the base branch (master) instead of the + # PR head. Without this override, lint runs against master and + # PR-side fixes (or breaks) are invisible to the check. + # Mirrors the pattern already used in build-android.yml / + # build-ios.yml / test-android-*.yml. + ref: ${{ github.event.pull_request.head.sha }} - name: Node uses: actions/setup-node@v4