Fixing tests for npm 2/3 by skevy · Pull Request #6346 · react/react-native

skevy · 2016-03-07T18:50:39Z

Supersedes #6343.

Note...this is just updating some Jest/node-haste configuration and the Shrinkwrap. The shrinkwrap update is what is key here.

Do not merge until CI passes :)

cc @mkonicek @bestander

…specific.

facebook-github-bot · 2016-03-07T18:51:30Z

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

skevy · 2016-03-07T20:11:28Z

🚢 🚢 🚢

mkonicek · 2016-03-07T21:55:51Z

@facebook-github-bot shipit

facebook-github-bot · 2016-03-07T21:56:31Z

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

mkonicek · 2016-03-08T00:31:31Z

Failed internal tests, will continue merging this with David tomorrow.

uncaught error Error: UnableToResolveError: Unable to resolve module ReactChildren from /data/.../react-native-github/Libraries/ReactNative/ReactNative.js: Unable to find this module in its module map or any of the node_modules directories under /data/node_modules/ReactChildren and its parent directories
This might be related to https://github.com/facebook/react-native/issues/4968
To resolve try the following:
1. Clear watchman watches: `watchman watch-del-all`.
2. Delete the `node_modules` folder: `rm -rf node_modules && npm install`.
3. Reset packager cache: `rm -fr $TMPDIR/react-*` or `npm start -- --reset-cache`.
at /data/.../react-native-github/node_modules/node-haste/lib/DependencyGraph/ResolutionRequest.js:413:17
at run (/data/.../react-native-github/node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:84:47)
at /data/.../react-native-github/node_modules/babel-polyfill/node_modules/core-js/modules/es6.promise.js:95:28
at flush (/data/.../react-native-github/node_modules/babel-polyfill/node_modules/core-js/modules/_microtask.js:19:5)
at nextTickCallbackWith0Args (node.js:419:9)
at process._tickCallback (node.js:348:13)
See logs /var/folders/4d/rqpj5cz94s557zqwa4csc4_m000xbj/T/react-packager.log
at SocketClient._handleMessage (SocketClient.js:144:23)
at BunserBuf.<anonymous> (SocketClient.js:53:42)
at emitOne (events.js:77:13)
at BunserBuf.emit (events.js:169:7)
at BunserBuf.process (/data/.../react-native-github/node_modules/bser/index.js:289:10)
at /data/.../react-native-github/node_modules/bser/index.js:244:12
at nextTickCallbackWith0Args (node.js:419:9)
at process._tickCallback (node.js:348:13)

bestander · 2016-03-08T13:20:10Z

Hey @skevy,

We are building the JS with BUCK and packager for internal apps and this error gets popped up:

uncaught error Error: UnableToResolveError: Unable to resolve module ReactDefaultPerf from /Users/bestander/fbsource/fbobjc/Libraries/FBReactKit/js/react-native-github/Libraries/Utilities/RCTRenderingPerf.js: Unable to find this module in its module map or any of the node_modules directories under /Users/node_modules/ReactDefaultPerf and its parent directories

The folder /Users/node_modules/ReactDefaultPerf looks weird, something is wrong with the paths.
@davidaurelio is going to have a look at this but if you have an idea where it could originate from, throw it here.

Right now we can't merge this PR.

bestander · 2016-03-08T13:40:00Z

The problem is probably in packager being executed not from react-native-github context (the folder that gets synced to this repository root) but from 4 folders up the tree

mkonicek · 2016-03-08T16:11:28Z

Just talked about this a bit more with David, the root cause is the difference in the internal and open source setup:

In open source React is a peer dependency of React Native
Internally React is a dependency of React Native, we don't have the peer dependency. We run npm shrinkwrap --dev (which includes React because it's a dev dependency of RN) and the create a tarball from node_modules and use that internally. We don't run npm on laptops of people who work on mobile apps, CI servers etc.

mkonicek · 2016-03-08T16:18:47Z

Also, React is a dev dependency of React Native, we run npm-shrinkwrap --dev to create the shrinkwrap file and we also publish it to npm.

This mean that anyone who runs react-native init gets two copies of React:
node_modules/react-native/node_modules/react, node_modules/react which is just wrong, regardless of whether React uses haste.

There's no easy way to change how the shrinkwrap generation works because we share that file both internally and in open source and the setup in each is very different.

We should stop publishing npm-shrinkwrap.json to npm, which means we'll only use that file internally as we have for the last year.

davidaurelio · 2016-03-08T16:40:20Z

I just realized that this cannot possibly work

The changes to node-haste (in 2.6) make incorrect assumptions:

the special haste-enabled folders live in <one-of-the-roots>/node_modules/<name> – that’s not the case for our internal setup, where stuff lives in <root>/a/b/node_modules/<name>
react lives in the top level node_modules folder. That’s not the case for our internal setup. React lives in <react-native-folder>/node_modules/react.

mkonicek · 2016-03-08T16:43:07Z

Thanks David, I'll close the PR.

@skevy you can reopen but don't see any way to merge this.

mkonicek · 2016-03-08T16:45:14Z

Should we revert some changes in node-haste?

mkonicek · 2016-03-08T16:46:00Z

Sorry it's probably not obvious at all how the internal setup works. I've only learned about it yesterday and today.

Summary:See comments on #6346 We cannot publish the shrinkwrap file to npm and make the same file work with the internal fb infra. We've never had the shrinkwrap file in a release of RN, keeping it that way for now should be ok. Reviewed By: bestander Differential Revision: D3024260 fb-gh-sync-id: 477509eb9a9890c1955a957ece922d692cd35aab shipit-source-id: 477509eb9a9890c1955a957ece922d692cd35aab

mkonicek · 2016-03-08T16:57:56Z

FYI I've removed shrinkwrap from what we publish to npm here: 10e990f

janicduplessis · 2016-03-08T19:54:04Z

It would be nice to publish a version of node-haste that has the Windows fix in it but not the haste module resolution changes to fix Windows support before the next release. Unless you think you can fix the bad assumptions so it work in your internal setup before that.

davidaurelio · 2016-03-08T21:45:04Z

Agreed. Let’s wait what @skevy thinks about is. I could publish node-haste 2.6.2 and remove the other 2.6.x releases

cpojer · 2016-03-08T21:47:42Z

Please let's just make sure we aren't breaking Jest again.

skevy · 2016-03-08T21:49:15Z

So, am I understanding right that we have to back out my changes to node-haste re isNodeModulesDir?

skevy · 2016-03-08T21:52:20Z

Ok yah, I guess we do. I still feel like we need to fix how that part of the packager works some how. Not sure if anyone has any ideas here.

davidaurelio · 2016-03-08T22:04:07Z

The best thing I can think of right now is not to use haste imports in react native at all, either by changing that directly in the source, or by rewriting it during npm publish

@cpojer that would be nice :-) I just checked, and jest still uses node-haste@^2.5.0. i.e. we should be good?

cpojer · 2016-03-08T22:07:21Z

@davidaurelio as long as we revert to whatever 2.5 did is fine. 2.6.0 breaks Jest pretty badly (and rn too I think).

davidaurelio · 2016-03-08T22:18:30Z

👍 I will be careful

skevy added 5 commits February 26, 2016 10:10

Update providesModuleNodeModule option to use new format and be more …
ba9bca9
…specific.

fixing jest config
54e9556

fixing quotes
b827773

Merge branch 'fix-providesModuleNodeModules' into fix-tests-03-07
9d9b7d8

fixing tests for npm2 and 3
332931e

skevy mentioned this pull request Mar 7, 2016
Attempt to make RN work with npm 2 #6343
Closed

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Mar 7, 2016

Merge remote-tracking branch 'upstream/master' into fix-tests-03-07
31b71c0

mkonicek mentioned this pull request Mar 7, 2016
Replace underscore by lodash #6030
Closed

facebook-github-bot added GH Review: accepted and removed GH Review: review-needed labels Mar 7, 2016

This was referenced Mar 7, 2016
Update providesModuleNodeModules option to use new format. #6039
Closed
Update node-haste and replace fast-path with node-haste's version to fix Windows compatibility #6260
Closed

davidaurelio added GH Review: needs-revision and removed GH Review: accepted labels Mar 8, 2016

mkonicek closed this Mar 8, 2016

Uh oh!

Conversation

skevy commented Mar 7, 2016

Uh oh!

facebook-github-bot commented Mar 7, 2016

Uh oh!

skevy commented Mar 7, 2016

Uh oh!

mkonicek commented Mar 7, 2016

Uh oh!

facebook-github-bot commented Mar 7, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

bestander commented Mar 8, 2016

Uh oh!

bestander commented Mar 8, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

davidaurelio commented Mar 8, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

mkonicek commented Mar 8, 2016

Uh oh!

janicduplessis commented Mar 8, 2016

Uh oh!

davidaurelio commented Mar 8, 2016

Uh oh!

cpojer commented Mar 8, 2016

Uh oh!

skevy commented Mar 8, 2016

Uh oh!

skevy commented Mar 8, 2016

Uh oh!

davidaurelio commented Mar 8, 2016

Uh oh!

cpojer commented Mar 8, 2016

Uh oh!

davidaurelio commented Mar 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants