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

RCTVideo re-registered direct event topReadyForDisplay as a bubbling event. #1618

Closed
paulrenenichols opened this issue Jun 11, 2019 · 16 comments

Comments

@paulrenenichols
Copy link

paulrenenichols commented Jun 11, 2019

Bug

image

Environment info

React native info output:

Also, this is running in the context of Expokit SDK 33.

info 
  React Native Environment Info:
    System:
      OS: macOS 10.14.5
      CPU: (12) x64 Intel(R) Core(TM) i9-8950HK CPU @ 2.90GHz
      Memory: 820.80 MB / 32.00 GB
      Shell: 5.3 - /bin/zsh
    Binaries:
      Node: 10.15.3 - ~/.nvm/versions/node/v10.15.3/bin/node
      npm: 6.4.1 - ~/.nvm/versions/node/v10.15.3/bin/npm
      Watchman: 4.9.0 - /usr/local/bin/watchman
    SDKs:
      iOS SDK:
        Platforms: iOS 12.2, macOS 10.14, tvOS 12.2, watchOS 5.2
      Android SDK:
        API Levels: 24, 25, 26, 27, 28
        Build Tools: 27.0.3, 28.0.2, 28.0.3
        System Images: android-Q | Google APIs Intel x86 Atom
    IDEs:
      Android Studio: 3.3 AI-182.5107.16.33.5314842
      Xcode: 10.2.1/10E1001 - /usr/bin/xcodebuild
    npmPackages:
      react: 16.8.3 => 16.8.3 
      react-native: https://github.com/expo/react-native/archive/sdk-33.0.0.tar.gz => 0.59.8 
    npmGlobalPackages:
      react-native-cli: 2.0.1
      react-native: 0.59.8

Library version: 4.4.1

Steps To Reproduce

No idea what is causing this.

Describe what you expected to happen:

Reproducible sample code

@AnderAmorim
Copy link

Same problem here :/

@CHaNGeTe
Copy link
Contributor

Do you have a list of RN libraries installed?
or see if onReadyForDisplay (searching in node_modules) is being used for more than one library? Just guessing...

@CHaNGeTe
Copy link
Contributor

The error is dismissable, right?

@paulrenenichols
Copy link
Author

@CHaNGeTe The error is dismissable, yes. We are using expo on this project, and one of my team members indicated that expo's video functionality seems to be causing the conflict here.

@annie-elequin
Copy link

update on this!

I was able to completely remove "EXAV" (which is expo-av in node_modules which is a piece of Expo) and removing that fixed the error.

Basically, react-native-video is declaring a type of listener that is named the same as a listener that expo-av declares (this is my understanding of the issue at least). So that's why removing expo-av fixes the problem.

Now, I haven't figured out how to exclude expo-av from being included when you're using expo (I think the solution is unimodules somehow but it's still getting pulled in) so when you "clean" the project (delete node modules, reinstall, update pods, etc) the problem comes back, and you would have to manually remove all the references to EXAV again.

If anybody has insight on the above ^ problem that would be super helpful.

@CHaNGeTe
Copy link
Contributor

@annie-elequin just curios, and I can not test it right now.
I see problem is on iOS, right?
Can you check if it happens with this branch?
https://github.com/24i/react-native-video/tree/feature/avoid-event-collision

@paulrenenichols
Copy link
Author

Thanks @CHaNGeTe. Let's test this branch out @annie-elequin

@cobarx
Copy link
Contributor

cobarx commented Jun 17, 2019

Nice find @CHaNGeTe. Didn't know DirectEventBlock was an option.

@annie-elequin
Copy link

Yeah that looks promising!

I tried it out, by adding this as the entry in package.json: git+https://github.com/24i/react-native-video.git#feature/avoid-event-collision

However, when I do that it's not getting all of the file from the branch (notably, .DRMType) which is causing it to fail.

image
image

@CHaNGeTe
Copy link
Contributor

Probably I branched from the wrong side, let me update the branch with only changes from master and let you know

@CHaNGeTe
Copy link
Contributor

try this one, notice the 3 in the end: https://github.com/24i/react-native-video/tree/feature/avoid-event-collision-3

@annie-elequin
Copy link

it worked! Thanks so much @CHaNGeTe

@danielmarino24i
Copy link
Contributor

Please @annie-elequin, can you test the updated branch? (I pushed some more changes)
Also I made a PR for this:
#1625

@CHaNGeTe
Copy link
Contributor

Fixed on 4.4.2

@jeremyfrancis
Copy link

This is still an issue.

@milicaNano
Copy link

Yeah that looks promising!

I tried it out, by adding this as the entry in package.json: git+https://github.com/24i/react-native-video.git#feature/avoid-event-collision

However, when I do that it's not getting all of the file from the branch (notably, .DRMType) which is causing it to fail.

image image

Where did you aded exactly?

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

No branches or pull requests

8 participants