chore: remove REACT_NATIVE_APP_ROOT, use --watchFolders

[grabbou]

Summary:

I don't think this is needed. Users can pass --watchFolders flag with the CLI (if they really need). They can also create their own metro.config.js where they define that value.

In my opinion, this is really confusing. I haven't found any places where it's referred to.

In other words, setting this:

REACT_NATIVE_APP_ROOT=path/to/cli node path/to/cli/packages/cli/build/index.js start

is equivallent to

node path/to/cli/packages/cli/build/index.js start --watchFolders path/to/cli 

(whether this makes sense or not is another topic)

[grabbou]

CC: @lithin I have a hard time understanding the use-case of that - can you please help me by bringing the issue you've run into and when this was a fix? That would help!

My understanding is that the location of the CLI doesn't matter. The projectRoot of the project is always the CWD.

And according to the Metro documentation:

Specify any additional (to projectRoot) watch folders, this is used to know which files to watch

Source: https://facebook.github.io/metro/docs/en/configuration#watchfolders

[grabbou]

Okay, looks like when this environmental variable is present, we will scan for symlinks by calling findSymlinkedModules - this performs a search inside node_modules.

Now, I haven't seen this referred to anywhere and I am not really sure what's the intent of this functionality.

Is there anyone here that has more knowledge of this particular feature?

[grabbou]

@thymikee also note that back in the day, we used to call resovleSymlinksForRoots in all cases - https://github.com/facebook/react-native/blob/0.55-stable/local-cli/util/Config.js#L54

Looks like this has changed and I don't know when. But it sounds like a regression.

[grabbou]

Okay, we used to call resolveSymlinksForRoots in all the cases, but it has been removed in this commit facebook/react-native@893c027 as a part of 0.57 release.

I am really confused right now because this change doesn't make any sense to me, unless Metro performs this functionally itself.

CC: @rafeca, I am hoping that you will be able to shed some light into this.

[grabbou]

Following up on this even further, this functionality has been introduced by @skevy in 2016 (lol) via facebook/react-native@39f83c4#diff-d939fe299a50129184bf6b4208f6d497

Apologies for pinging both of you on such historical piece of code, but... does this mean symlinks used to work with React Native and could've been accidentally removed via facebook/react-native@893c027 commit?

I always believed that symlinks never worked and seeing symlinks related code always made me confused.

[rafeca]

Hey @grabbou ! 👋

It's hard to remember the intention of my commit there LOL (don't know if there's more context in D9444982 - I can't access diffs anymore), but it's quite probably that facebook/react-native@893c027 removed the resolveSymlinksForRoots call by mistake 😅

[grabbou]

CC: @rickhanlonii would you mind helping out and checking some context on the D9444982? Thanks @rafeca for answering!

[rickhanlonii]

The only thing in the diff is this comment you left @rafeca:

From what I see, we could actually just not set watchFolders, since right now it's only set to the value of REACT_NATIVE_APP_ROOT if it exists (which is an env variable that I cannot find in any RN docs).

I think we could safely revert the change as long as we confirm that facebook/react-native#20712 doesn't regress 👍

[thymikee]

@grabbou tested this locally and running --watchFolders on yarn-linked project still works. Updated the PR to remove undocumented REACT_NATIVE_APP_ROOT env instead.
cc @brentvatne as this env is used by Expo XDL. This change will affect RN 0.62+