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

Issue adding support for Bridgeless, Lottie React Native #42477

Closed
TheRogue76 opened this issue Jan 21, 2024 · 15 comments
Closed

Issue adding support for Bridgeless, Lottie React Native #42477

TheRogue76 opened this issue Jan 21, 2024 · 15 comments
Labels
Newer Patch Available Resolution: Fixed A PR that fixes this issue has been merged. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@TheRogue76
Copy link
Contributor

TheRogue76 commented Jan 21, 2024

Description

Hi there.

I have been working on adding bridgeless support for Lottie React Native, and i have hit a rather strange issue.
When i export my fabric component using

export default codegenNativeComponent<NativeProps>('LottieAnimationView') as HostComponent<NativeProps>

The app succeeds in running in bridgeless mode, and shows the UI and the Fabric events are sent back correctly, but i get a console.error: Native Component 'LottieAnimationView' that calls codegenNativeComponent was not code generated at build time. Please check its definition (I have traced this console error to here in React Native internals)
But it runs correctly and both shows the Fabric component and sends back the correct events

 (NOBRIDGE) ERROR  Native Component 'LottieAnimationView' that calls codegenNativeComponent was not code generated at build time. Please check its definition.
 (NOBRIDGE) LOG  Running "example" with {"rootTag":1,"initialProps":null,"fabric":true}
 (NOBRIDGE) LOG  Finished

Looking at the generated LottieAnimationView, the output seems correct to me as far as i can tell.
if i change the signature to

export default codegenNativeComponent<NativeProps>('LottieAnimationView')

The bridgeless console error goes away, but it throws an error when the event needs to come back:

 (NOBRIDGE) ERROR  Error: Unsupported top level event type "topAnimationFinish" dispatched

What am i missing here? The only way i have found to register the events without this error has been to put them in customDirectEventTypes from react-native/Libraries/Renderer/shims/ReactNativeViewConfigRegistry, but that looks like a very internal API, and from what i understood it registers it at the global scope, which is not ideal because someone else might also have the same event name or some other thing might happen

Steps to reproduce

  1. Clone the repo
  2. run yarn in the root
  3. cd apps/fabric
  4. run bundle install && RCT_NEW_ARCH_ENABLED=1 pod update --project-directory=ios
  5. run yarn start
  6. build the app, either iOS or Android (Feel free to use Xcode or the commands in package.json)
  7. Observe the error

React Native Version

0.73.1

Affected Platforms

Runtime - Android, Runtime - iOS

Areas

Bridgeless - The New Initialization Flow

Output of npx react-native info

System:
  OS: macOS 14.2.1
  CPU: (10) arm64 Apple M1 Max
  Memory: 66.53 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 20.10.0
    path: ~/.nvm/versions/node/v20.10.0/bin/node
  Yarn:
    version: 3.2.3
    path: ~/.yarn/bin/yarn
  npm:
    version: 5.1.0
    path: ~/Desktop/lottie-react-native/node_modules/.bin/npm
  Watchman: Not Found
Managers:
  CocoaPods:
    version: 1.14.0
    path: /Users/Parsa/.rvm/gems/ruby-3.1.2/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.2
      - iOS 17.2
      - macOS 14.2
      - tvOS 17.2
      - visionOS 1.0
      - watchOS 10.2
  Android SDK:
    API Levels:
      - "26"
      - "28"
      - "29"
      - "30"
      - "31"
      - "32"
      - "33"
      - "34"
    Build Tools:
      - 28.0.3
      - 29.0.2
      - 30.0.2
      - 30.0.3
      - 31.0.0
      - 32.0.0
      - 32.1.0
      - 33.0.0
      - 33.0.1
      - 34.0.0
    System Images:
      - android-33 | Google APIs ARM 64 v8a
      - android-33 | Google Play ARM 64 v8a
      - android-34 | Google APIs ARM 64 v8a
      - android-34 | Google Play ARM 64 v8a
    Android NDK: Not Found
IDEs:
  Android Studio: Not Found
  Xcode:
    version: 15.2/15C500b
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.7
    path: /usr/bin/javac
  Ruby:
    version: 3.1.2
    path: /Users/Parsa/.rvm/rubies/ruby-3.1.2/bin/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.73.1
    wanted: 0.73.1
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

(NOBRIDGE) ERROR  Native Component 'LottieAnimationView' that calls codegenNativeComponent was not code generated at build time. Please check its definition.
 (NOBRIDGE) LOG  Running "example" with {"rootTag":1,"initialProps":null,"fabric":true}
 (NOBRIDGE) LOG  Finished

Reproducer

https://github.com/lottie-react-native/lottie-react-native/tree/feature/add-experimental-bridgeless

Screenshots and Videos

Running in Bridgeless mode, with the default export cast as HostComponent<NativeProps>
Screenshot 2024-01-21 at 11 26 03
Screenshot 2024-01-21 at 12 01 58
Running in Bridgeless mode, WITHOUT the default export cast as HostComponent<NativeProps>
Screenshot 2024-01-21 at 11 36 32

@TheRogue76 TheRogue76 added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Jan 21, 2024
Copy link

⚠️ Missing Reproducible Example
ℹ️ We could not detect a reproducible example in your issue report. Please provide either:
  • If your bug is UI related: a Snack
  • If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

@github-actions github-actions bot added Needs: Author Feedback Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. labels Jan 21, 2024
Copy link

⚠️ Newer Version of React Native is Available!
ℹ️ You are on a supported minor version, but it looks like there's a newer patch available - 0.73.2. Please upgrade to the highest patch for your minor or latest and verify if the issue persists (alternatively, create a new project and repro the issue in it). If it does not repro, please let us know so we can close out this issue. This helps us ensure we are looking at issues that still exist in the most recent releases.

@TheRogue76
Copy link
Contributor Author

⚠️ Missing Reproducible Example
ℹ️ We could not detect a reproducible example in your issue report. Please provide either:

  • If your bug is UI related: a Snack
  • If your bug is build/update related: use our Reproducer Template. A reproducer needs to be in a GitHub repository under your username.

The reproducer is not under my name, but the package itself. Ignoring this error.

@github-actions github-actions bot added Needs: Attention Issues where the author has responded to feedback. and removed Needs: Author Feedback labels Jan 21, 2024
@TheRogue76
Copy link
Contributor Author

The pull request that is currently in the works is https://github.com/lottie-react-native/lottie-react-native/pull/1160/files so if you do not wish to read the whole thing, just the changes, you can read them there

@TheRogue76
Copy link
Contributor Author

@cortinico @cipolleschi (Sorry for pinging you guys directly) any thoughts and help on this is appreciated. I guess the Issue tracker is not pinging since it is still saying there is no reproducible.

@cortinico
Copy link
Contributor

We discussed this internally briefly. We'll get back to you in the next future to see what we can do to support 👍

@TheRogue76
Copy link
Contributor Author

Sounds good. Putting this on hold for now

@dmytrorykun
Copy link
Contributor

(NOBRIDGE) ERROR  Native Component 'LottieAnimationView' that calls codegenNativeComponent was not code generated at build time. Please check its definition.

👆This issue has been already fixed by adding support for the as expression for TypeScript: https://github.com/facebook/react-native/blob/main/packages/babel-plugin-codegen/index.js#L87-L95
The fix should be available with react-native 0.74. Alternatively you can try the latest nightly version of @react-native/babel-plugin-codegen, the fix is there.

 (NOBRIDGE) ERROR  Error: Unsupported top level event type "topAnimationFinish" dispatched

👆And I'm currently investigating this.

@TheRogue76
Copy link
Contributor Author

TheRogue76 commented Jan 23, 2024

Thank you @dmytrorykun. I guess that means there is no plans to backport this change for 0.73?

@dmytrorykun
Copy link
Contributor

@TheRogue76 I guess we should do this, yeah.
reactwg/react-native-releases#98 (comment)

@dmytrorykun
Copy link
Contributor

@TheRogue76 the quick fix would be to delete the 'onAnimationFinish', 'onAnimationFailure', and 'onAnimationLoaded' arguments from here. I'm currently investigating the underlying problem.

@TheRogue76
Copy link
Contributor Author

TheRogue76 commented Jan 23, 2024

@dmytrorykun That's intriguing. I'll test it out locally. Please keep me posted on the investigation.
Update: works like a charm.

@TheRogue76
Copy link
Contributor Author

Hi @dmytrorykun, is it ok if i update the PR? Now that 73.3 is out i want to move things forward, but i would totally understand if you need more time with this. I can redo this in another PR so that you would have the branch to investigate freely as well

@dmytrorykun
Copy link
Contributor

Hi @TheRogue76 , sorry, I forgot to post the update here. This other issue has also been fixed, and pick is requested for 0.73.4 (reactwg/react-native-releases#101 (comment)).

@TheRogue76
Copy link
Contributor Author

Hi @TheRogue76 , sorry, I forgot to post the update here. This other issue has also been fixed, and pick is requested for 0.73.4 (reactwg/react-native-releases#101 (comment)).

Perfect! Thank you! Marking the issue as resolved, since every variant has been addressed.

@cortinico cortinico added Resolution: Fixed A PR that fixes this issue has been merged. and removed Needs: Triage 🔍 Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. Needs: Attention Issues where the author has responded to feedback. labels Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Newer Patch Available Resolution: Fixed A PR that fixes this issue has been merged. Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

3 participants