Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make tests pass using Node 5 / NPM 3 #832 #840

Closed
wants to merge 22 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
language: node_js
node_js:
- '4'
- '5'
env:
- TEST_DIR=.
- TEST_DIR=scripts/babel-relay-plugin
Expand Down
6 changes: 4 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
"dependencies": {
"babel-runtime": "5.8.24",
"fbjs": "^0.7.0",
"react-static-container": "^1.0.0-alpha.1"
"react-static-container": "^1.0.0-alpha.1",
"babel-relay-plugin": "0.7.1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea is that people would need to add babel-relay-plugin to their project with npm 3, so it should remain (just) a peer dependency. I'd love someone with more npm knowledge here..

cc @steveluscher

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit: If the plugin isn't under dependencies, you have to manually install it with npm 3.
Which means npm test will fail in travis-ci since the plugin wasn't installed. You can see the failing tests above for one of my commits: https://travis-ci.org/facebook/relay/builds/109438390

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just guessing here, but should it maybe be in devDependencies in that case? It's needed to run the unit tests, but if you're using it in your own project, you should add it to your dependencies (to use it in your transform step).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure, let me try it out
On Fri, Feb 19, 2016 at 6:28 PM Jan Kassens notifications@github.com
wrote:

In package.json
#840 (comment):

@@ -30,7 +30,8 @@
"dependencies": {
"babel-runtime": "5.8.24",
"fbjs": "^0.7.0",

  • "react-static-container": "^1.0.0-alpha.1"
  • "react-static-container": "^1.0.0-alpha.1",
  • "babel-relay-plugin": "0.7.1"

Just guessing here, but should it maybe be in devDependencies in that
case? It's needed to run the unit tests, but if you're using it in your own
project, you should add it to your dependencies (to use it in your
transform step).


Reply to this email directly or view it on GitHub
https://github.com/facebook/relay/pull/840/files#r53542588.

},
"peerDependencies": {
"babel-relay-plugin": "0.7.1",
Expand Down Expand Up @@ -92,7 +93,8 @@
"<rootDir>/node_modules/fbjs/lib/(?!(ErrorUtils.js$|fetch.js$|fetchWithRetries.js$))",
"<rootDir>/node_modules/react/",
"<rootDir>/node_modules/react-dom/",
"<rootDir>/node_modules/react-static-container/"
"<rootDir>/node_modules/react-static-container/",
"<rootDir>/node_modules/(?!(fbjs/lib/ErrorUtils.js$|fbjs/lib/fetch.js$|fbjs/lib/fetchWithRetries.js$))"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you keep the deleted lines here and just have 2 patterns and add 4 and 5 to the travis config, do the tests pass in both environments? It'd be nice to work in both if it's just adding the new patterns instead of replacing them.

Is there something that makes it difficult to keep the npm 2 patterns in this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only difficulty is I don't know how to. Let me see what I can do

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should be able to revert this change once I update to jest 0.9.0-fb3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok sure! I'll be awaiting jests 0.9.0-fb3 release

]
}
}
1 change: 1 addition & 0 deletions scripts/babel-relay-plugin/src/.flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

[include]
../node_modules/
../../../node_modules/util/

[libs]
../interfaces
Expand Down
2 changes: 2 additions & 0 deletions src/.flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
.*/node_modules/graphql/.*
.*/react/node_modules/.*
.*/react-static-container/node_modules/.*
.*/babel-relay-plugin/node_modules/.*

[include]
../node_modules/fbjs/lib
../node_modules/react
../node_modules/react-static-container/lib
../node_modules/base62

[libs]
../node_modules/fbjs/flow/lib
Expand Down