-
Notifications
You must be signed in to change notification settings - Fork 904
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
Android: latest release broke support custom react-native Root #804
Comments
cc @grabbou |
AFAICT the change was intentional, to fix a lot of other custom structure issues. Now, if the RN app is nested in a directory, we expect users to call CLI from that nested directory, and we don't care whether the node_modules/react-native are hoisted or not. Please share your repo structure and the way you use it, so we can advise. |
Sure. appreciate in advance your guidance. I am showing quite a bit of complexity below (essentially most of our front-end project tree). Then you will see There is a lot of co-dependency across all of that stuff and reliance on the specific directory structure we have now.
|
Thanks for reporting this over. The problem here is that you don't have The CLI automatically resolves the "root" of your project by looking for the closest I am surprised that this setup is working for you. Autolinking is looking for dependencies to link from a "package.json" and there's clearly no "package.json" with modules to link in your setup. As a result of such directory structure, the two following problems will happen:
The solution requires doing few tweaks to the directory structure you have set up. Yarn workspacesThe most elegant way would be to set up a Yarn workspaces project for this. In order to get this working, you would need to make sure that each project has a valid Your You also need to explicitly list down all React Native packages that you want to link to your Android/iOS app in such When running It will also automatically support setting cross-links between your projects. Just set your packages to depend on each other and Yarn will take care of it. For example, our CLI uses this and packages are automatically symlinked when needed. It is also the setup that we recommend for "monorepositories" and library developers. We are most likely to keep supporting it in the foreseeable future. |
One additional note regarding my previous comment: I believe you have designed this folder structure to re-use code between mobile and web, hence "rn.common" folder with "common" code. However, taking a closer look at its contents, it's not "common". It contains "index.android.js" (entry point for an Android mobile app) and many other things. So at the end of the day, it combines two things: React Native code for Android mobile application and some React / JavaScript code that you may share on the Web. In my opinion, the React Native code responsible for Android / iOS mobile application should reside under "a/u1b" where the complementary native code is located. That way, you can write some mobile-specific code as well without polluting the common folder. By creating a |
@grabbou thank you for the follow ups. Our Android app is a 'hybrid' (there is only one activity using RN), and we already specify the packages that must be 'loaded' by the RN Android runtime (there is only 1, vector-icons). Given that you spend effort offering explanation on why we should restructure our repo (thank you for that), let me offer some replies (although I am not sure they are particuallary convicing, I hope it gives bit more background/picture to others as well). Overall, once in a while we think of moving our 'common/reusable' code into some form of custom node modules, then setting up custom NPM-like repo internally and pulling it from there. Or learning other tooling options around lerna/etc.
It appears, that the change was made, exactly in this feature. Perhaps, you were thinking to model it after how webpack looks up node_modules. But in many cases, RN is just small piece of the overall edge-device/front-end code and the overall structure is dictated by other 'opinioned/less opinioned' toolchains. So overall, I would still advocate that autolinking or any other RN build toolchain feature allows for flexibility of defining where files can reside, and were roots of RN can reside.
Well that's was not the intent. We are not that RN-centric. We would put 2nd android app under a, but not under a/u1b So a/u1b means 'AndroidApp-endusers-Domain1B' As a philosophy, in a way, we structure our monorepo on combination of 'BusinessFunction+BusinessDomain', rather than on 'ProgrammingLanguage+ProgrammingLibrary+BussinessFunction' We also have operations domains (devops, and so on, they are structured same way, but start with devops as this a operations domain).
That's true, I tried my best to move android.index.js (and 2 other files) out of rn.common into its own. But even if metro would have fixed the above issue, moving index.android.js out of rn.common would not have helped the new CLI to find rn.common. Not sure if this useful, but here is (sanitizied) output of When it is executed from rn.common (this is what CLI was receiveing, when it was respecting my root setting, with the old code
|
@thymikee app/build.android into line 155 of the npx command in
then it will work. |
Looking at you project structure, this patch should work in your case. With the changes done there we'd run Am I missing something? Feel free to replace your |
@thymikee I just tested your update on top of 0.61, and it works for us!. It seems that what you have done, is leveraged the fact that everybody in their That works well, it makes sense, it allows us the flexibility to keep RN common code, in whatever directory structure we want, and, as 'icing on the cake' eliminates the need to re-specify the location of that directory like 3 additional times. |
Reopening because of #852. We need to find better solution |
@grabbou I have a monorepo structure set up almost identical to the one you suggested earlier, but the RN CLI doesn't work properly in root. I don't want a I checked and I'm sure that Looking through the global Structure just to be clear:
where the sub-projects have essentially this as their package.json
and root has ideally nothing in its dependencies (or very little). Also please feel free to send me to another issue if this isn't a good place to ask. |
why did you closed this issue |
@thymikee I am getting React native 0.62. when executing the build from android studio
The problem still seems to be same (using 4.7.0 release of cli. Tried both on Windows and Linux). My settings.gradle is the same as when reported the issue 6 months ago
Same build.gradle:
NODE_MODULES environment variable is not set v12.16.1 The problem, clearly the same as before When settings.gradle invokes To confirm that this is same problem, I modified native_modules.gradle
That allowed to build to pass the above mentioned error... But without the above change to CLI code, I simply do not know how to get this to even start the build. |
This comment has been minimized.
This comment has been minimized.
still happen in 0.71.3 |
Environment
System:
OS: Windows 10
CPU: (8) x64 Intel(R) Core(TM) i
Memory: 15.44 GB
Binaries:
Node: 12.10.0 - C:\Program Files\nodejs\node.EXE
Yarn: 1.19.1 - C:\Users\v\AppData\Roaming\npm\yarn.CMD
npm: 6.12.0 - C:\Program Files\nodejs\npm.CMD
npmPackages:
react: 16.9.0 => 16.9.0
react-native: ^0.61 => 0.61.2
Description
my directory structure is
andrapp\ <-- has android ./app , gradlew and so on
rn.common\ <-- has package.json index.android.js and ./node_modules
in react-native 60.6 I was able to specify the React native root (in app/build.gradle)
And cli was able to correctly invoke
npx.cmd --quiet react-native config
from within rn.common directory
However since this change
5724d29#diff-375026a0607eb034a6fc70cca7d74689
native_modules.gradle
Now invokes npx in 'default' directly, not in React's project root. Which causes
npx.cmd --quiet react-native config
to complain with
Which, then, causes JSON parsing error in native_modules.js
Here is code change in native_modules.js that causes the error on Android
// old code, that was working
cmdProcess = Runtime.getRuntime().exec(command, null, root)
// new code that does not work
cmdProcess = Runtime.getRuntime().exec(command)
From the design/documentation perspective, it seems that the whole infrastructure for getReactNativeProjectRoot() have been removed, and no alternative/migration documentation has been suggested.
Note also, that react-native 0.61 project template, continues to assert that configuring custom root (where package.json) resides, by specifying root in project.ext.react (in app/build.gradle) -- is still a valid and supported option
https://github.com/facebook/react-native/blob/master/template/android/app/build.gradle
The text was updated successfully, but these errors were encountered: