Monorepo Support: Use node resolver for react native location #2337
Conversation
|
|
||
| // Use node resolver to locate react-native package | ||
| def reactNativePackage = file(["node", "--print", "require.resolve('react-native/package.json')"].execute(null, projectDir).text.trim()) | ||
| if (reactNativePackage.exists()) { |
There was a problem hiding this comment.
Inspect with:
println reactNativePackage.parentFile
You'll then see this output in the terminal when you run yarn android again.
|
I'm able to run the same command locally that is failing in CI. I've also dropped back to node 18.20.4 locally to see if that made a difference. Everything continues to build normally. It appears that for some reason |
There was a problem hiding this comment.
Hello, I've finally found time to test this, thank you for the contribution 🎉 We will surely merge this!
First, however, I need to articulate my confusion regarding differences between old and new approach.
Honestly I do not understand, why does the current main code even work. In context of react-native-screens, $projectDir in "regular client react-native application" resolves to App/node_modules/react-native-screens/android, thus "$projectDir/../node_modules/react-native/android" resolves to App/node_modules/react-native-screens/node_modules/react-native/android => This look like we've been pointing to wrong react-native module over the last couple of years!? What's even more baffling is that when you inspect App/node_modules/react-native-screens/node_modules/react-native it surely does not contain android subdirectory... I'm sure there are mechanisms I'm just now aware now as of right now, but that's just crazy ;D
Okay, so looking at the solution you propose I think we're fine on the first part, and the fallback to "new-old" approach with rootDir instead of projectDir also does make more sense than starting point old approach, at least for now.
I've tested this on:
- newly created standard react native app & it seems to be working,
- our examples in the library repository - they work just fine.
Only remark that my purist brain brings up is that maybe we should first look for "standard" application structure and if it fails, just then try to handle other cases with fallback code. Basically swap the cases around, so that we do not run whole search algorithm if it is not needed.
I'm thinking this through right now & I believe such swap won't break any behaviour you expect in monorepo, hence I'll allow myself to change it. Let me know, if I'm in wrong and the order is important.
There is one more thing to do: revert changes to guides/CONTRIBUTING.md file. You are right - these guidelines are wrong, however your suggestion is also wrong, despite the fact that yarn prepare commands exists in our repo :D I'll fix this in separate PR.
Since I can't push to your fork I'll close this PR, cherry-pick your changes and open a new one.
|
Surrogate PR: #2352 |
## Description Based on: #2337 by @morganick. Please see the original PR and its description for details. See: #2337 (review) for discussion why new PR has been created. ## Changes - **Using node resolver to find react native package for better monorepo support** - **Using rootDir instead of projectDir** - **Change the order of path lookup** ## Test code and steps to reproduce I've tested it using our both example apps and additionally I've created fresh RN app and tested the Android build there. ## Checklist - [x] Ensured that CI passes --------- Co-authored-by: Nick Morgan <[email protected]>
## Description Based on: software-mansion#2337 by @morganick. Please see the original PR and its description for details. See: software-mansion#2337 (review) for discussion why new PR has been created. ## Changes - **Using node resolver to find react native package for better monorepo support** - **Using rootDir instead of projectDir** - **Change the order of path lookup** ## Test code and steps to reproduce I've tested it using our both example apps and additionally I've created fresh RN app and tested the Android build there. ## Checklist - [x] Ensured that CI passes --------- Co-authored-by: Nick Morgan <[email protected]>
## Description Based on: #2337 by @morganick. Please see the original PR and its description for details. See: #2337 (review) for discussion why new PR has been created. ## Changes - **Using node resolver to find react native package for better monorepo support** - **Using rootDir instead of projectDir** - **Change the order of path lookup** ## Test code and steps to reproduce I've tested it using our both example apps and additionally I've created fresh RN app and tested the Android build there. ## Checklist - [x] Ensured that CI passes --------- Co-authored-by: Nick Morgan <[email protected]> (cherry picked from commit 408112a)
Description
Monorepos are configured and structured on what works best for the project. Since this can vary from project to project, having a dynamic way to find dependencies will make this work for more projects. Currently, the repo only supports certain configurations of Monorepo structures. This PR adopts a similar approach as Expo to locate the react native package within the project. Expo usage for reference
Changes
build.gradleto have node resolve the react native package directoryCONTRIBUTING.mdto correct setup step asyarn preparedoes not exist inside ofreact-navigationsubmodule.Test code and steps to reproduce
printlnstatement in thebuild.gradlefile to examine the output ofreactNativePackage.parentFileChecklist
Included code example that can be used to test this changeUpdated TS types