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

[Fabric] Fix Fabric component & TM specs for new architecture #877

Merged

Conversation

shwanton
Copy link
Contributor

@shwanton shwanton commented Apr 2, 2024

Summary

We use this library internally at Meta and have run into some build issues when upgrading & using it with the new architecture.

  • The Fabric component spec is required to have "NativeComponent" as a suffix, not prefix.

The file must be named <MODULE_NAME>NativeComponent, with a .js or .jsx extension when using Flow, or a .ts, or .tsx extension when using TypeScript. Codegen only looks for files matching this pattern.

fabric-native-components.md

  • Fabric component spec shouldn't create a ComponentDescriptor since one is added manually.

https://github.com/react-native-datetimepicker/datetimepicker/blob/master/ios/fabric/cpp/react/renderer/components/RNDateTimePicker/ComponentDescriptors.h

  • TM Codegen was including a typedef from a separate file, which is not supported.
  • Android Codegen does not support typed parameters to codegen'd methods.
  • "RCTFabricComponentsPlugins.h" is used internally and needs to be scoped to <React/RCTFabricComponentsPlugins.h>

Test Plan

Fabric - iOS

CleanShot.2024-04-02.at.16.20.26.mp4

Paper - iOS

CleanShot.2024-04-02.at.16.44.15.mp4

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Build w/ new arch enabled:
RCT_NEW_ARCH_ENABLED=1 pod install

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)
  • I added a sample use of the API in the example project (example/App.js)
  • I have added automated tests, either in JS or e2e tests, as applicable

@shwanton shwanton changed the title Newarch fixes 0 72 [Fabric] Fix Fabric component & TM specs for new architecture Apr 2, 2024
@arushikesarwani94
Copy link

New Arch Disabled - Android

Paper.mov

New Arch Enabled[Fabric + TurboModule] - Android

NewArchSmall.mov

@shwanton shwanton marked this pull request as ready for review April 3, 2024 02:41
@vonovak vonovak merged commit bd7c078 into react-native-datetimepicker:master Apr 4, 2024
5 checks passed
vonovak pushed a commit that referenced this pull request Apr 4, 2024
## [7.6.4](v7.6.3...v7.6.4) (2024-04-04)

### Bug Fixes

* **fabric:** fix Fabric component & TM specs for new architecture ([#877](#877)) ([bd7c078](bd7c078))
@vonovak
Copy link
Member

vonovak commented Apr 4, 2024

🎉 This issue has been resolved in version 7.6.4 🎉

If this package helps you, consider sponsoring us! 🚀

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

Successfully merging this pull request may close these issues.

3 participants