Skip to content
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.

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

Fix child argument types in addView methods #857

Merged

Conversation

szydlovsky
Copy link
Contributor

@szydlovsky szydlovsky commented Aug 19, 2024

Summary

Fixes #850 and #856.
As stated in the first issue, there seem to be unneeded nullabilities in child arguments for Android PagerViewManager (both paper and fabric). Their existence makes the methods override nothing, which in turn crashes the build.

Test Plan

Run on a fresh android app.

What's required for testing (prerequisites)?

Having anything react-native-pager-view related in the app.

What are the steps to reproduce (after prerequisites)?

You need to create 2 differents apps - one with RN 0.74, the other with RN 0.75 - to make sure we don't lose <0.75 compatibility. Then, after installing dependencies just try building via Android Studio.

Compatibility

OS Implemented
iOS not relevant
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)

@saif-o99
Copy link

Someone please accept this PR. its urgent fix

Copy link
Member

@troZee troZee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it work for RN < 75 and the old arch?

@saif-o99
Copy link

there is also addView inside main/PagerViewViewManagerImpl.kt
this this also needs to change?

@szydlovsky
Copy link
Contributor Author

@troZee checked for RN 0.74.4 on both old and new arch - worked fine. 0.75.1 both arches work as well

@szydlovsky
Copy link
Contributor Author

and yeah, PagerViewViewManagerImpl.kt might also need some tweaks I believe, but everything builds and works fine without it

@szydlovsky
Copy link
Contributor Author

@troZee should I also add changes to the Implementation files?

@troZee troZee merged commit 993928c into callstack:master Aug 19, 2024
3 checks passed
@hengkx
Copy link

hengkx commented Aug 19, 2024

Thank you very much. When will the version be released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'addView' overrides nothing error with React Native 0.75
5 participants