use lookup folder instead of cwd for symlink path resolution. Refs #9009

[ekryski]

Explain the motivation for making this change. What existing problem does the pull request solve?

Fixes the symlink path resolution. See discussion here: #9009 (comment)

Test plan (required)

Symlink a required module by running npm link in the module directory and then npm link <module name> in your project and run npm start. The packager and server should start properly and not throw an error about not being unable to find the symlink.

cc/ @Kureev @bestander

[ghost]

By analyzing the blame information on this pull request, we identified @Kureev to be a potential reviewer.

[ekryski]

I did notice that although this resolves the symlink path issue, when the packager is resolving dependencies it's still unable to load my symlinked files. I still need to copy them into the project's node_modules/<my module> path in order to avoid the red screen of death.

Might try and dig into that later today to see. @Kureev have you gotten symlinks working with your fixes? Were they just JS modules symlinked in or were did they have native pieces as well? I'd be curious to see a simple example if so.

[ekryski]

So after doing a lot of digging, even though my PR works, @Kureev's previous work and this fix is only step 1 in resolving symlinks out of the box with RN. Basically #637 is still not solved.

Currently, the packager won't actually resolve symlinks if you are using watchman due to this issue: facebook/watchman#105.

After following the rats nest of dependency injection in the packager I've traced through what happens.

With my fix the symlinked modules are now being included in the haste map but not in the the Fastfs cache. The reason they aren't being included in the Fastfs cache is because when the RN server starts up, we initially crawl the file system for dependencies and the packager calls out to the file system watcher (which by default is watchman) to find the dependencies for the project. Since watchman does not know how to resolve symlinks they are never returned in the file list that gets shoved into Fastfs.

So... because the files are in the haste map but are never included in Fastfs.js, this line always returns undefined, resulting in a missing module error when trying to load the JS bundle. Since _getFile is called internally for pretty much every function in Fastfs.js symlinks are never really resolved ever.

Tracing Through

1. The main server starts and initializes a bunch of stuff.
2. node-haste initializes a crawler
3. Fastfs gets the files from the crawler and caches them
4. Haste Map, Asset Map & Dependency Graph get built, seemingly from the Fastfs cache. This is where I'm confused as I haven't figured out why the packager is even attempting to look up symlink paths in Fastfs later on. They aren't in the cache, so I don't know how they are ending up in the haste map.
5. When loading the JS bundle, the server asks the Bundler for the JS bundle and the Bundler calls out to other modules to get all the dependencies and transform them.
   • Bundler asks Resolver for dependencies
   • Resolver asks node-haste for dependencies.
   • node-haste creates a ResolutionRequest and asks for ordered dependencies
   • which eventually calls resolveDependency from this line
   • which calls _resolveHasteDependency
   • which finally calls _loadAsDir (or _loadAsFile) and this is where things fall down because Fastfs doesn't have the symlinked dependency.

At least that's how I think it works 😄 . Apologies for the brevity, that was more for me because it's quite complicated to trace through.

Possible Solutions

After looking through all the related issues I have some proposed solutions:

Use the fallback node watcher

This seems like a simple and great solution for local development. However, given the number of dependencies with React Native V8 ends up running out of memory. Basically the fallback node watch is a no go. So it's not really a solution. I just put it here because it has been mentioned to just "brew uninstall watchman". That doesn't really work.

Fix Watchman to support Symlinks

Progress is being made here but it's slow and it seems like it's pretty complex. Way out of my element here.

WML

@dutzi wrote a utility called wml that can be used to watch files and copy them. So far I think this is the best solution.

Copy-paste hacks

Much like wml you could write your own custom script to copy your files over to the correct destination when they change. I'm currently bending babel's watch command to my will as an npm script.

babel --watch -d /path/to/project/node_modules/symlinked_module/ /path/to/symlinked_module/src/

Any insight or suggestions on how we can fix this would be really helpful and greatly appreciated. I'm totally willing to help. The current solutions for developing local modules just aren't working well with teams and multiple modules. Even though @dutzi's wml works well it's yet another tool that people in the React Native community need to know and worry about (no knock to you, thanks for doing that @dutzi!).

cc @wez @Kureev @bestander @davidaurelio @dutzi @amasad

[bestander]

@ekryski, thanks for such a good and insightful investigation.

1. Watchman.
   Last time I checked we can't make watchman support symlinks.
   The reason is in a bug in MacOS file system and that it completely breaks file change events.

It might be possible to support file symlinks instead of folder symlinks, not sure if it is any useful for our case.

There is a hope that the new Mac OS version will fix the bug and then we might have it afterall.
Also people work on other kinds of file systems that will resolve this issue eventually.

1. Node-watch

It sucks that it crashes for RN codebase.
Can we do anything about it?
Is there a bug open?

1. WML

How about integrating WML solution into React Native?
Just a thought from the top of my head, packager could replace symlinks in node_modules replace with wml, that could be somewhat seamless.

[bestander]

As for the PR, the change looks good to me.
I'll let @Kureev have a look.
If he is not available, ping me and I'll merge this PR

[Kureev]

LGTM.

Currently, the packager won't actually resolve symlinks if you are using watchman due to this issue: facebook/watchman#105.

What's the difference with "normal" watchman usage? I've tested it on a new project after PR has been merged and it worked for me. Do you have a very specific setup?

[Kureev]

@bestander is it fine that travis fails?

[bestander]

Yes, Travis has some infra problems

[ekryski]

What's the difference with "normal" watchman usage? I've tested it on a new project after PR has been merged and it worked for me. Do you have a very specific setup?

@Kureev I've got a package that I am symlinking into an example app in the same repo. I am attempting to use npm link to bring the module being developed into the example app. So the structure looks like this:

-> example-app
   -> ios/
   -> android/
   -> node_modules/
   -> index.ios.js
   -> index.android.js
   -> package.json
-> node_modules/
-> lib/ (built out by babel)
-> src/ (module being developed)
-> package.json

This is simplified but that's the general structure. I'll try it out on a fresh app with a really basic symlinked package. @Kureev are you able to share your test app?

[ekryski]

@bestander

Watchman. Last time I checked we can't make watchman support symlinks. The reason is in a bug in MacOS file system and that it completely breaks file change events. It might be possible to support file symlinks instead of folder symlinks, not sure if it is any useful for our case.

Bummer. Ya I don't know if that will help either. My hunch is it won't.

Node-watch. It sucks that it crashes for RN codebase. Can we do anything about it? Is there a bug open?

Not sure. I can poke around a bit but I think the only way would be to reduce the number of files/dependencies in RN. There are a lot. We are talking about the watcher processing ~3000 files.

Maybe there is a hack to tell node-watcher to allocate more memory as that might be a bandaid to avoid the out of memory exceptions. In any case it's still seems slower than watchman by quite a bit.

How about integrating WML solution into React Native? Just a thought from the top of my head, packager could replace symlinks in node_modules replace with wml, that could be somewhat seamless.

I had been thinking about that as well. Similar to what happened with rnpm. Have it as a command or have it auto-detect symlinked files. WML blew up a bit with my folder structure above (symlinked module being up one directory from where it is used) but when it is in a folder such that it is a sibling to the example app it works great.

The other solution I was thinking of is that we could have a different watcher (maybe node-watcher) just for symlinked modules. Maybe we'd still have memory issues (especially if the external modules are big or there are a lot of them).

I was also looking to see if you could set up watchman to watch the symlinked directory and tell it to copy files to the projects node_modules directory (which is basically what wml is doing), so using WML might be the best solution really.

The process being:

1. detects a symlink
2. registers original linked directory/file with watchman or wml, with the output directory as the project's node_modules
3. run the second watcher or wml so that the linked folder/files are copied to the output directory

The way the packager looks for paths it should pick it up without issue because it will just think that the module is installed as a normal dependency.

The only potential hiccup is that npm link and npm install behave a little differently. npm link doesn't use .npmignore and .gitignore files (as far as I can tell, could be wrong here). So you might end up with a bunch of junk copied over or files that should be excluded from the watch (.git/, node_modules, dotfiles, etc.).

[bestander]

The other solution I was thinking of was could be have a different watcher (maybe node-watcher) just for symlinked modules.

That may work as a quick compromise

[Kureev]

To repeat my testing scenario:

1. Run react-native init MyAwesomeApp
2. Clone react-native-<name> somewhere(I placed it as a sibling of MyAwesomeApp) and npm link it
3. Go to MyAwesomeApp and npm link react-native-<name>
4. Run react-native run-ios/android

[deoqc]

tl;dr: Shouldn't this merged asap so people who already had symlink in node_modules - for example universal js - don't run in ENOENT error in startup? (I had this issue, more precise use case bellow).

---

I already used symlinks in universal js (for server and tests), and was able to mimic the symlink behaviour with simple package.json like pointed here.

But this wrong symlink searching path made the app don't start with ENOENT error. And I if I changed the symlink to work with react-native would break the server and tests.

[bestander]

Ok, let's merge this for now.
Feel free to come up with a better solution for that, I think we will iterate over this in the coming months.

[bestander]

@facebook-github-bot shipit

[ghost]

Thanks for importing. If you are an FB employee go to Phabricator to review internal test results.

[ghost]

I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification.

[bestander]

This file was changed in #9819.
Could you rebase?

[ekryski]

Sorry I was MIA for a bit there.

PR #9792 actually does an even better job of symlink resolution by using recursion to handle nested symlinks as well. So I think we can just close this actually since that has been merged.