WIP: build: accept option for react-native project path

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.

[tbergquist-godaddy]

Summary:

In monorepos, the root path, where node_modules live
might not be the same as where the actual react-native project is.

Test Plan:

I created a new react-native project, ran yarn android, and it still works. I tested it from our monorepo (sorry, it is closed source) and it works. I can provide you with open-source monorepo to test it later if necessary.

I still consider this to be work in progress. I want to do similar changes also for iOS.
Also, I would like to use commander for reading the rn-project-path, or if there is a better way, please suggest.

As for the commander, I still don't understand it fully, since when I try to read the command args before the loadConfig call, the program exits.

Also, maybe add some tests if it makes sense

closes#717

[tbergquist-godaddy]

From what I can see, iOS does not call react-native config does it?

If it does, where exactly does it call it?

[thymikee]

iOS uses config as well: https://github.com/react-native-community/cli/blob/master/packages/platform-ios/native_modules.rb#L14

Thanks for sending the PR :). I'll try to process it next week. We'd like to make a better story for monorepos and custom directory structures (there's a lot of open issues and PRs trying to address that, but we need to think it through). I'm not yet sure if it's gonna be by adding an extra flag, we'll see.

[tbergquist-godaddy]

iOS uses config as well

Ah, yes it does 💡, but for my project setup, this does not need to change, since it is already called where my react-native-project is, and that works.

Which is probably why I thought I was not called at all 😊

[tbergquist-godaddy]

Ok, I am beginning to understand that this problem is a little bit more complex.

The changes that I did so far allows me to build the android app, which without these changes crashes. However, images from 3rd party libraries are missing, like react-navigation-stack back button image.

On iOS it was building correctly and all assets like the back button is correctly in the development version. But they are missing in production build version 🤔

[grabbou]

Hm, I have a feeling we have seen (or actually had) previous attempts on supporting monorepos in this project. What was the outcome of these?

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

I also wonder how other tools approach this problem.

[tbergquist-godaddy]

I think what needs to be solved is:

• In which path to run react-native config (this PR)
• How to make sure bundle assets are added correctly, both for dev & production.

I don't understand why 3rd party assets are correctly bundled in on iOS for dev, but not for android. What are the platform differences there?

Also, I don't understand why these 3rd party differences disappears on iOS production build.

I was thinking that any additional configuration like that (e.g. node_modules location) could be part of the configuration if that makes any sense.

Maybe something like this. The metro bundler already has watchFolders, which makes it possible to tell metro which other folders to bundle code from. Maybe something like this is needed for the cli as well 🤷‍♂

[grabbou]

In which path to run react-native config (this PR)

I am wondering if we shouldn't leverage the cosmiconfig ability to look for the configuration up the tree.

Cosmiconfig continues to search up the directory tree, checking each of these places in each directory, until it finds some acceptable configuration (or hits the home directory).

In that case, the place you're running react-native config from wouldn't essentially matter. Right now, we have specified the stopDir property, making it not look outside of the cwd.

If we would let it look up the tree, running it from packages/mobile would read it in packages/mobile, packages and . (and possibly even further up the tree, but that's probably acceptable trade-off).

This is aligned with require.resolve AFAIK and shouldn't cause problems when a command is executed at a different level than node_modules.

What do you think?

[grabbou]

The big question here is: why we haven't turned it on by default?

I remember trying it out and hitting some issues. I guess it would be a good starting point to try to do it and see what happens (and observe our test suite blow up)

[tbergquist-godaddy]

Yeah, that sounds good.

[tbergquist-godaddy]

So, if that is the preferred approach, please feel free to close this PR.

[grabbou]

All righty! Do you plan on working on that approach? I'd love to help (e.g. via Discord) if you have capacity. If not, don't worry!

[tbergquist-godaddy]

I will try to have a look at it tomorrow (not promising anything), and I will let you know.

Is there a channel for this on discord, do you have a link?

[grabbou]

Are you on React Native Community Discord? There's #cli channel there. If not, I can issue you an invite.

[tbergquist-godaddy]

Please send an invite 😊

[grabbou]

Actually my bad, we already look for the configuration file recursively. Then I guess I need to better understand the root cause.

Sending

[grabbou]

@tbergq, I have sent another PR #769 to remove the recursive reading of the configuration.