Make providesModuleNodeModules behavior to be more deterministic.

[skevy]

Currently, the providesModuleNodeModules option allows for specifying an array of package names, that the packager will look for within node_modules, no matter how deep they're nested, and treat them as packages that use the @providesModule pragma.

In reality, this is not actually the behavior we want.

NPM, because of how it handles dependencies, can do all kinds of arbitrary nesting of packages when npm install is run. This causes a problem for how we deal with providesModuleNodeModules. Specifically...take fbjs. Currently, React Native relies on using the Haste version of fbjs (will be resolved in #5084). Thus if npm installs multiple copies of fbjs...which is a very common thing for it to do (as can be seen by this list of issues: https://github.com/facebook/react-native/issues?utf8=%E2%9C%93&q=naming+collision+detected), we get into a state where the packager fails and says that we have a naming collision.

Really, the behavior we want is for the packager to treat only a single copy of a given package, that we specify in the Resolver in the providesModuleNodeModules option, as the package that it uses when trying to resolve Haste modules.

This PR provides that behavior, by changing providesModuleNodeModules from a list of strings to a list of objects that have a name key, specifying the package name, as well as a parent key. If parent is null, it will look for the package at the top level (directly under node_modules). If parent is specified, it will use the package that is nested under that parent as the Haste module.

To anyone who reads this PR and is familiar with the differences between npm2 and npm3 -- this solution works under both, given the fact that we are now shipping the NPM Shrinkwrap file with React Native when it's installed through npm. In both the npm2 and npm3 case, node_modules specified by RN's package.json are nested underneath react-native in node_modules, thus allowing us to specify, for example, that we want to use React Native's copy of fbjs (not any other copy that may be installed) as the module used by the packager to resolve any requires that reference a module in fbjs.

Test Plan:

npm 2

1. react-native init a new project
2. Using npm@2, npm install --save skevy/react-native#fix-providesModuleNodeModules react@0.14.5
3. Observe the project compiling correctly, despite multiple copies of react and fbjs being installed.

npm 3

1. react-native init a new project
2. Using npm@3, npm install --save skevy/react-native#fix-providesModuleNodeModules react@0.14.5
3. Observe the project compiling correctly, despite multiple copies of react and fbjs being installed.

/cc @martinbigio@mkonicek@bestander@davidaurelio@ide

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @amasad, @cpojer and @skevy to be potential reviewers.

[mkonicek]

Wow that's a lot of issues :( https://github.com/facebook/react-native/issues?utf8=%E2%9C%93&q=naming+collision+detected

First things I asked people on the JS infra team when I joined RN was why Haste existed at all. The answer is the same system worked really well for us in PHP - you can move files around without having to codemod thousands of files importing them for example.

Thanks for doing this as well as #5084.

[skevy]

@mkonicek to be clear, I love Haste. We use it at Exponent on internal projects, and I'm working on a project that let's me use it with Node.

However, I think where it shines, and why it works for Facebook, is that it's suited particularly to product code. The issue, and the "hard part", is that we have a packager that (currently) has to understand both systems.

I know that it's the goal of people (@spicyj, @cpojer, and others) to remove the magic from the packager. This PR specifically is really meant as a stop gap, and to make the behavior that's currently there make a bit more sense and produce more desirable behavior. The end goal, of course, is that this option wouldn't need to exist at all...@providesModule code could be used within RN itself, and within any product code that depends on RN...but we wouldn't have to worry about consuming Haste-enabled code from npm, which is where everything breaks down (at least without some type of namespacing for Haste modules...which is not currently implemented (and I'm not sure anyone at FB even has a desire for that feature)).

[martinbigio]

Thanks for implementing this so quickly!

[martinbigio]

@facebook-github-bot shipit

[facebook-github-bot]

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1680514035538734/int_phab to review.

[skevy]

@ide lol.

[skevy]

@martinbigio can you hold this for the moment? I need to fix some weird issue with the obj-c/android tests.

[martinbigio]

sure

[ide]

@skevy lol sorry about that. my chrome extension must have gone off -_-

[facebook-github-bot]

@skevy updated the pull request.

[facebook-github-bot]

@skevy updated the pull request.

[sophiebits]

Does this work if you install fbjs and react-native as siblings? Also, it seems like maybe you could use path.resolve instead of manually concatenating node_modules strings but if this works, it works.

[skevy]

Ok, @martinbigio, I think if you take a look at this and are OK with it we should be good to go.

I refactored isNodeModulesDir. The process now is that, instead of having isNodeModulesDir do a bunch of path manipulation, we build an array of the actual resolved paths to the modules specified in providesModuleNodeModules, and then in isNodeModulesDir, we simply check whether or not the file given to the function is in any of those directories (and not in any of their nested node_modules, because again, we only want one copy).

@spicyj yes, this works if fbjs and react-native are siblings...because now we guarantee that we're using the version of fbjs that RN wants as it's Haste module provider.

[facebook-github-bot]

@skevy updated the pull request.

[skevy]

In a holding pattern on this until facebookarchive/node-haste#33 is resolved and merged...then I'll update this PR accordingly with any RN specific changes (most likely will just be the update to the providesModuleNodeModules option in Resolver).

[facebook-github-bot]

@skevy updated the pull request.

[facebook-github-bot]

@skevy updated the pull request.

[facebook-github-bot]

@skevy updated the pull request.

[bestander]

Any ETA on this one?
We want to get our Travis stable.
Maybe provide a rollback while you sort out the haste logic?

[facebook-github-bot]

@skevy updated the pull request.