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

Fix build_codegen! not finding @react-native/codegen in pnpm setups #1983

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

tido64
Copy link
Member

@tido64 tido64 commented Nov 13, 2023

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary:

build_codegen! currently assumes that @react-native/codegen gets installed next to react-native. In a pnpm setup, it's found under /~/react-native/node_modules/@react-native/codegen instead.

However, as @dmytrorykun pointed out, we don't actually need to build it outside of this repository.

Cherry-picks facebook@3dd6a83

Changelog:

[GENERAL] [FIXED] - @react-native/codegen shouldn't be built unless it's in the repo — fixes pod install failures in pnpm setups

Test Plan:

We have a patched version of react-native working in a pnpm setup here: microsoft/rnx-kit#2811

`build_codegen!` currently assumes that `@react-native/codegen` gets
installed next to `react-native`. In a pnpm setup, it's found under
`/~/react-native/node_modules/@react-native/codegen` instead.

However, as @dmytrorykun pointed out, we don't actually need to build
it outside of this repository.
@tido64 tido64 merged commit d5092cd into 0.72-stable Nov 17, 2023
16 of 17 checks passed
@tido64 tido64 deleted the tido/0.72/codegen-build branch November 17, 2023 07:48
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.

2 participants