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

feat: disable RNTA temporarily #658

Merged
merged 9 commits into from
Nov 8, 2024
Merged

feat: disable RNTA temporarily #658

merged 9 commits into from
Nov 8, 2024

Conversation

atlj
Copy link
Collaborator

@atlj atlj commented Oct 21, 2024

Summary

  1. Disables the RNTA temporarily
  2. Mentions you need to add package.json to your exports in the docs
  3. Automatically adds package.json with bob if you don't have includesGeneratedCode: true.
  4. Closes codegen doesn't find the library #637

Why the changes?

Currently, there are two approaches with React Native Codegen.

1. Pregenerate

If you have includesGeneratedCode: true in your codegenConfig, you can generate the Codegen specs in the build time and ship them with your library. This is the recommended way.

2. Let the app generate

If you don't have includesGeneratedCode: true, when the application builds, the React Native Codegen is invoked and goes through all the dependencies of the app and generates the codegen specs. However, if you have ESModule exports and if you haven't added your package.json to the exports property, then the Codegen silently skips your library.


Because Codegen silently fails when you don't have package.json in the exports or don't build Codegen specs at the build time, people might have big problems figuring out this on their own. So we're temporarily disabling RNTA until we enable build time spec generation for it.

Test plan

CRNL

  1. Generate a library using crnl
  2. Make sure the RNTA isn't an option there

Bob

  1. Do bob init on a library
  2. Make sure package.json is added to the exports field.

@atlj atlj changed the title @atlj/packagejson not defined feat: disable RNTA temporarily Oct 21, 2024
@atlj atlj mentioned this pull request Oct 21, 2024
2 tasks
@tido64
Copy link

tido64 commented Oct 23, 2024

Hey @atlj, I'm just trying to understand how RNTA leads to this bug and what we can do to forward fix it instead of disabling it. Afaics, isn't this a bug in core?

@@ -295,9 +295,14 @@ yargs
},
};

if (pkg.codegenConfig && !pkg.codegenConfig.includesGeneratedCode) {
// @ts-expect-error The exports is not strictly types therefore it doesn't know about the package.json property
exportsField['./package.json'] = './package.json';
Copy link

Choose a reason for hiding this comment

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

This field should always be present, imo. There are tools, like linters, that reads package.json for various reasons.

@atlj
Copy link
Collaborator Author

atlj commented Oct 23, 2024

Hey @tido64 thanks for jumping in, we don't have any problems with RNTA itself right now. The root problem is that we haven't implemented the precompiled codegen spec shipping feature for RNTA yet, which is currently causing some fragmentations. The plan is to disable the RNTA option temporarily and bring it back with full precompiled codegen spec shipping.

If you're curious about what we need to implement in CRNL for this, you can take a look at #566 where we implemented this feature for vanilla example apps.

@atlj atlj force-pushed the @atlj/packagejson-not-defined branch from 6e6ab63 to dfb84d9 Compare October 26, 2024 12:47
value: 'test-app',
description: "app's native code is abstracted away",
},
// The test app is disabled for now until proper
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

add env var to enable this

@@ -108,6 +112,25 @@ yarn add --dev react-native-builder-bob

> If you're building TypeScript definition files, also make sure that the `types` field points to a correct path. Depending on the project configuration, the path can be different for you than the example snippet (e.g. `lib/typescript/index.d.ts` if you have only the `src` directory and `rootDir` is not set).

1. Configure [React Native Codegen](https://reactnative.dev/docs/the-new-architecture/what-is-codegen)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

move this to bottom

@@ -108,6 +112,25 @@ yarn add --dev react-native-builder-bob

> If you're building TypeScript definition files, also make sure that the `types` field points to a correct path. Depending on the project configuration, the path can be different for you than the example snippet (e.g. `lib/typescript/index.d.ts` if you have only the `src` directory and `rootDir` is not set).

1. Configure [React Native Codegen](https://reactnative.dev/docs/the-new-architecture/what-is-codegen)

You can follow the [Official Codegen Setup Guide](https://reactnative.dev/docs/the-new-architecture/using-codegen) to enable Codegen.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

mention when you need to configure codegen

@atlj atlj force-pushed the @atlj/packagejson-not-defined branch from b73b7c3 to 56d467c Compare November 8, 2024 18:01
@atlj atlj merged commit aa400f6 into main Nov 8, 2024
27 of 29 checks passed
@atlj atlj deleted the @atlj/packagejson-not-defined branch November 8, 2024 18:03
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.

codegen doesn't find the library
2 participants