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

codegen - javaPackageName is ignored with includesGeneratedCode and react-native codegen #45079

Open
mfazekas opened this issue Jun 20, 2024 · 8 comments
Assignees
Labels
Impact: Bug The issue represents a bug somewhere Never gets stale Prevent those issues and PRs from getting stale Tech: Codegen Related to react-native-codegen Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)

Comments

@mfazekas
Copy link
Contributor

Description

react-native codegen ignores javaPackageName in package.json/codeGenConfig/andorid/javaPackageName and uses com.facebook.fbreact.specs

Steps to reproduce

See https://github.com/mfazekas/rn-codegen-javapackagename

See

generateSpecsCLIExecutor.generateSpecFromInMemorySchema(
platform,
schemaInfo.schema,
tmpOutputDir,
libraryName,
'com.facebook.fbreact.specs',
schemaInfo.library.config.type,
useLocalIncludePaths,
);

React Native Version

0.74.2

Affected Platforms

Runtime - Android

Areas

Codegen

Output of npx react-native info

% npx react-native info
info Fetching system and libraries information...
System:
  OS: macOS 14.5
  CPU: (12) arm64 Apple M2 Max
  Memory: 118.64 MB / 32.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.20.2
    path: ~/.nvm/versions/node/v18.20.2/bin/node
  Yarn:
    version: 3.6.4
    path: ~/.nvm/versions/node/v18.20.2/bin/yarn
  npm:
    version: 10.5.0
    path: ~/.nvm/versions/node/v18.20.2/bin/npm
  Watchman:
    version: 2024.05.06.00
    path: /opt/homebrew/bin/watchman
Managers:
  CocoaPods:
    version: 1.14.2
    path: /Users/boga/.rbenv/shims/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2024.1 AI-241.15989.150.2411.11948838
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 20.0.1
    path: /usr/bin/javac
  Ruby:
    version: 2.7.8
    path: /Users/boga/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.2
    wanted: 0.74.2
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: Not found
  newArchEnabled: false

Stacktrace or Logs

n/a

Reproducer

https://github.com/mfazekas/rn-codegen-javapackagename/

Screenshots and Videos

No response

@mfazekas mfazekas added Needs: Triage 🔍 Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules) labels Jun 20, 2024
@cortinico
Copy link
Contributor

This is (sadly) a known bug. The issue is happening regardless of you using includesGeneratedCode or not.

I have an internal ticket to work on this.
Sadly fixing it would be a breaking change for every codegen user so we're being a bit caution on when we should fix.

@cortinico cortinico added Tech: Codegen Related to react-native-codegen Impact: Bug The issue represents a bug somewhere Never gets stale Prevent those issues and PRs from getting stale and removed Needs: Triage 🔍 labels Jun 20, 2024
@cortinico cortinico self-assigned this Jun 20, 2024
@mfazekas
Copy link
Contributor Author

@cortinico thanks much.

Another issue is that outputDir is not relative to package root, but to cwd. So in the reproducer if run:

cd ReproducerApp
npx react-native codegen --path ../RTNCalculator --platform android

it generates into ./android/codegen which is ReproducerApp/android/codegen instead of ../RTNCalculator/android/codegen.

RTNCalculator/package.json:

    "includesGeneratedCode": true,
    "outputDir": {
      "android": "android/codegen"
    }

Is this a bug? Shall I open a separate issue for it?

@cortinico
Copy link
Contributor

it generates into ./android/codegen which is ReproducerApp/android/codegen instead of ../RTNCalculator/android/codegen.

mmm I'm not sure this follows to your previous statement.
If as you said, the path is relative to cwd (being ./ReproducerApp in your example), then stuff should be generated inside:

./RTNCalculator/android/codegen

no?

@mfazekas
Copy link
Contributor Author

mmm I'm not sure this follows to your previous statement. If as you said, the path is relative to cwd (being ./ReproducerApp in your example), then stuff should be generated inside:

./RTNCalculator/android/codegen

no?

Sorry if I wasn't clear. I'd expect

"includesGeneratedCode": true,
    "outputDir": {
      "android": "android/codegen"
    }

to always generate into RTNCaclulator/android/codegen so always into the lib, but if I run from the app directory it creates new folder in the app

cd ReproducerApp
npx react-native codegen --path ../RTNCalculator --platform android

it generates into ReproducerApp/android/codegen

ReproducerApp % ls -la android/codegen/java/com/rtncalculator 
total 8
drwxr-xr-x  3 boga  staff    96 Jun 20 14:24 .
drwxr-xr-x  4 boga  staff   128 Jun 20 14:24 ..
-rw-r--r--  1 boga  staff  1130 Jun 20 14:24 NativeRTNCalculatorSpec.java

@cortinico
Copy link
Contributor

@dmytrorykun can help you more here as he implemented the react-native codegen command. But yes this is probably a separate bug which needs a separate issue

@dmytrorykun
Copy link
Contributor

This is definitely a bug. I'll take a look

@mfazekas
Copy link
Contributor Author

@dmytrorykun opened issue #45112

@atlj
Copy link
Contributor

atlj commented Jul 5, 2024

We've been hitting this issue in react-native-builder-bob. I've added a script to patch this behavior there. You can find the script here
For folks who want to use this, when we land this in bob, you will be able to call it with --target codegen and it should generate the codegen specs and patch this.

github-merge-queue bot pushed a commit to callstack/react-native-builder-bob that referenced this issue Aug 16, 2024
## Summary

So far with the new architecture-supported templates, we've been
generating libraries that didn't ship their codegen-generated specs.
This means the library's users had to build the codegen specs on their
end (this was done implicitly). With this, the codegen-generated specs
are generated at the build time and they are shipped with the library.

I've followed [this guide from the React Native new arch working
group](https://github.com/reactwg/react-native-new-architecture/blob/main/docs/codegen.md#including-generated-code-into-libraries).

### Making sure example app builds are triggering codegen

An important problem to figure out was to make sure the
codegen-generated specs were being built with each native build of the
example app. To do that,`create-react-native-library` now modifies
non-legacy example apps and adds:

1. A new task to `example/android/build.gradle` that's triggered before
each native build.
2. A prebuild action to the  XCode build schema.
3. A `pre_install` hook to `example/ios/Podfile` that's triggered when
user calls `pod install`.

These modifications make sure `yarn codegen` is called on the repo root
to generate the codegen specs.

### Notes

1. There is an important problem with React Native itself right now.
When `react-native codegen` is called, the generated Java code doesn't
follow the `codegenConfig.android.javaPackageName` property in the
`package.json`. This means the generated Java files are stored in a
default location with the wrong package name. To fix it, I've added a
script that moves the codegen-generated files into the correct place.
You can check facebook/react-native#45079 to
see more.

## Test plan

### Test if Android builds trigger codegen

1. Create a non-legacy library using `create-react-native-library`
2. Install the dependencies and build the example app on Android
3. Make sure the build passes, and there are files under
`android/generated`.

### Test if installing pods triggers codegen

1. Create a non-legacy library using `create-react-native-library`
2. Install the dependencies and run `pod install` in `example/ios`
3. Make sure the pods are installed and there are files under
`ios/generated`.

### Test if iOS builds trigger codegen

1. Create a non-legacy library using `create-react-native-library`
2. Install the dependencies and run `pod install` in `example/ios`
3. Remove the codegen generated code from `ios/generated` since that's
generated by the pod install step
4. Build the app for iOS
5. Make sure there are files in `ios/generated`.

### Test if building the library triggers codegen

1. Run `yarn prepare` to emulate the library release process
2. Run `yarn pack`
3. Extract the files from the generated `package.tgz`
4. Make sure there are files under `ios/generated`, and
`android/generated` in the generated package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact: Bug The issue represents a bug somewhere Never gets stale Prevent those issues and PRs from getting stale Tech: Codegen Related to react-native-codegen Type: New Architecture Issues and PRs related to new architecture (Fabric/Turbo Modules)
Projects
None yet
Development

No branches or pull requests

4 participants