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] Make sure that RCTEventEmitter is registered before emitting the first event #809

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

cipolleschi
Copy link
Contributor

Summary

React Native pager view emits an error when used with Nightlies on the New Architecture with and without the Bridge.

The reason is that Pager View copied a work around we also have in ScrollView to allow apps to implement complex animations, like parallax animations, but it forgot to add the JS side of it, that makes the workaround effective.
The JS part is to make sure that RCTEventEmitter is properly registered as CallableModule in the JS side.

This PR adds that bit, copying what ScrollView does.

For context, we are not happy with this workaround. We still have to figure out properly how to fix it and, as soon as we know, we are going to provide guidance and/or submit a new PR to fix this for good, depending on how much time we will have

Test Plan

Tested locally using:

Before After
Simulator Screen Recording - iPhone 15 - 2024-02-13 at 18 00 38 Simulator Screen Recording - iPhone 15 - 2024-02-13 at 17 59 10

What's required for testing (prerequisites)?

Being able to create and run a react native app.

What are the steps to reproduce (after prerequisites)?

npx react-native@nightly init AppName --skip-install --version nightly
cd AppName
yarn add react-native-pager-view
cd ios
bundle install
RCT_NEW_ARCH_ENABLED=1 bundle exec pod install
open NewApp.xcworkspace
cd ..
yarn start

then:

  1. open the App.tsx
  2. copy-and-paste the example in the react-native-pager-view github repo
  3. try to swipe and observe the error.

Compatibility

OS Implemented
iOS
Android Not needed

Checklist

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

@troZee troZee merged commit 8d064d4 into callstack:master Mar 29, 2024
2 checks passed
@WoLewicki
Copy link
Contributor

I think we want to revert it for RN 0.75 support since facebook/react-native#44474 is available there?

@MrRefactor
Copy link
Collaborator

I will revert it with next release supporting 0.75

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.

4 participants