Skip to content

fix: Remove nullability of command id #983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Apr 22, 2025
Merged

fix: Remove nullability of command id #983

merged 1 commit into from
Apr 22, 2025

Conversation

MatiPl01
Copy link
Contributor

Summary

This PR applies necessary changes for RN 0.79. Before this change the library won't compile on android on RN 0.79. This change is needed because of this commit in react-native.

Test Plan

  • clone this repo (clean app with RN 0.79-rc.3 and latest react-native-pager-view)
    • git clone https://github.com/MatiPl01/react-native-pager-view-issues.git
  • cd react-native-pager-view-issues && git switch 0.79-android-build
  • build the app for android and observe the error (Argument type mismatch: actual type is 'kotlin.String?', but 'kotlin.String' was expected.)
  • install library witch changes from this PR
    • npm install git+https://github.com/MatiPl01/react-native-pager-view.git#fix/remove-nullability-of-command-id-arg
  • build the app for android again and see that build succeeds

Compatibility

OS Implemented
iOS ✅
Android ✅

Checklist

  • I have tested this on a device and a simulator
  • I added the documentation in README.md
  • I updated the typed files (TS and Flow)

@MatiPl01 MatiPl01 mentioned this pull request Mar 28, 2025
Merged
@MatiPl01
Copy link
Contributor Author

Can someone possibly take a look at this? cc: @troZee

@pipo151086
Copy link

any advance in this issue?

@MrRefactor
Copy link
Collaborator

Will check that

@MrRefactor MrRefactor self-requested a review April 6, 2025 14:05
@iam-rohid
Copy link

Hi @MrRefactor. why is it stuck at Link package? When will this PR get merged? I'm having issues build our project after upgrading to expo@canary and React 19.

file:///private/var/folders/78/nmsqtd_n1lzfqlkcvqjtld_h0000gn/T/eas-build-local-nodejs/5f865ff1-0b0f-4962-a45e-918c2e2892ec/build/node_modules/react-native-pager-view/android/src/fabric/java/com/reactnativepagerview/PagerViewViewManager.kt:41:40 Argument type mismatch: actual type is 'kotlin.String?', but 'kotlin.String' was expected.

@MrRefactor
Copy link
Collaborator

Hi @MrRefactor. why is it stuck at Link package? When will this PR get merged? I'm having issues build our project after upgrading to expo@canary and React 19.

file:///private/var/folders/78/nmsqtd_n1lzfqlkcvqjtld_h0000gn/T/eas-build-local-nodejs/5f865ff1-0b0f-4962-a45e-918c2e2892ec/build/node_modules/react-native-pager-view/android/src/fabric/java/com/reactnativepagerview/PagerViewViewManager.kt:41:40 Argument type mismatch: actual type is 'kotlin.String?', but 'kotlin.String' was expected.

Will release new version tomorrow, as for now you can patch this

@iam-rohid
Copy link

HI @MrRefactor. Let's merge this PR?

@argshook
Copy link

Hola, waiting for the PR too. Would close #988

Manual patching works, but requires extra steps for CI.

@NiuGuohui
Copy link
Contributor

In fact, on the Fabric new architecture, we can directly remove the rewrite of receiveCommand, because the parent class has a better implementation.

@TheSon-devv
Copy link

@MrRefactor Please help us merge this

@MrRefactor MrRefactor merged commit 482da0b into callstack:master Apr 22, 2025
2 checks passed
@MrRefactor
Copy link
Collaborator

released in 6.7.1

@leonhh leonhh mentioned this pull request Apr 23, 2025
Merged
3 tasks
vonovak pushed a commit to expo/expo that referenced this pull request Apr 23, 2025
@leonhh
update react-native-pager-view to 6.7.1 (#36324)
61dae50 
# Why

Version 6.7.1 fixes an Android build error:
callstack/react-native-pager-view#983

# How

Bump version in package.json files and bundledNativeModules.json.
Removed patch.

# Test Plan

Tested in bare-expo

# Checklist

<!--
Please check the appropriate items below if they apply to your diff.
-->

- [ ] I added a `changelog.md` entry and rebuilt the package sources
according to [this short
guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MrRefactor MrRefactor MrRefactor approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@MatiPl01 @pipo151086 @MrRefactor @iam-rohid @argshook @NiuGuohui @TheSon-devv