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

Trim react-native from the name in the same places create-rn-library and Codegen does #123

Merged
merged 3 commits into from
Oct 15, 2024

Conversation

jhugman
Copy link
Owner

@jhugman jhugman commented Oct 11, 2024

Fixes #109.

This PR adds testing around the turbo-modules.

The tests runs through a host of different names, then

  • runs create-react-native-library with that name.
  • runs the ubrn generate turbo-module command
  • the checks that generated files from the ubrn command still uses the identifiers in the create-react-native-library files
  • finally builds for ios and android the library.

This has meant a series of changes (for the better) to the templates.

@jhugman jhugman force-pushed the jhugman/109-fixup-react-native-trimming branch 4 times, most recently from 5cd24f4 to 75ad20e Compare October 11, 2024 01:39
@jhugman
Copy link
Owner Author

jhugman commented Oct 11, 2024

I haven't yet got the test running in CI, but I think that can wait for another PR. Marking ready for review.

@jhugman jhugman changed the title Add testing for turbo-modules generated code Trim react-native from the name in the same places create-rn-library and Codegen does Oct 11, 2024
@jhugman jhugman requested a review from zzorba October 11, 2024 10:33
@@ -264,7 +264,7 @@ mod files {
templated_file!(IndexTs, "index.ts");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this also end in .tsx?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, you're right. I renamed the template file and made this index.tsx to match. I also renamed the IndexTs struct.

Thanks for prompting me out of laziness. :)

@jhugman jhugman force-pushed the jhugman/109-fixup-react-native-trimming branch from 75ad20e to 07855bd Compare October 12, 2024 17:32
@jhugman jhugman force-pushed the jhugman/109-fixup-react-native-trimming branch from 07855bd to 47d5ef0 Compare October 12, 2024 17:36
Copy link
Collaborator

@zzorba zzorba left a comment

Choose a reason for hiding this comment

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

LGTM

@jhugman jhugman merged commit c3fd7ab into main Oct 15, 2024
1 check passed
@jhugman jhugman deleted the jhugman/109-fixup-react-native-trimming branch October 15, 2024 15:07
jhugman added a commit that referenced this pull request Oct 16, 2024
Fixes #124 

According to [The Big O of Code
Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a
O(_n_) change.

This adds to the tests introduced in #123 to support names with an org
name.
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.

Generated code is being prefixed (e.g. with "ReactNative") but some occurrences appear to be missing
3 participants