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.

Sign up for GitHub

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

Drop Node 9 support #1445

Merged
merged 1 commit into from
Aug 2, 2018
Merged

Drop Node 9 support #1445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 0 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ language: node_js
# https://github.com/nodejs/Release
node_js:
- '10'
- '9'
- '8'
- '6'

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"mocha": "--check-leaks --full-trace --timeout 15000 src/**/__tests__/**/*-test.js"
},
"engines": {
"node": "6.x || 8.x || 9.x || >= 10.x"
"node": "6.x || 8.x || >= 10.x"
Copy link
Contributor

@leebyron leebyron Aug 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I'm curious to see if this ends up creating issues. This engines line means something quite different than the travis config.

Travis config means we only explicitly test support for a certain set of node versions. This engines line means we require these versions of node to operate - otherwise warnings are emitted during installation.

v9 is not an experimental branch, and there are plenty of companies that sanely use the latest version of node. The difference between odd and even numbered versions is that odd numbered versions have short support cycles while even have long ones. Out of LTS just means it wont get security patches anymore

Copy link

Choose a reason for hiding this comment

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

Well, came here from figuring out an issue. :) For reasons, we're depending on an older version of nan which fails to compile in Node v10 (but worked fine in v9). And we're upgrading to graphql 14.0.0 to get extended inputs. In any case, I'm not sure why this change is necessary - is v9 missing a feature that graphql depends on? Additionally, I don't think I've seen other modules limit engines in this way - i.e. removing a node version because it's no longer in maintenance - but limiting it because of missing features makes sense. 2c.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jure I removed node 9 because one of our dev dependencies stoped supporting it:
https://github.com/amasad/sane/blob/49f251a95e6db118ad71223b1eca93c876d9aacc/package.json#L47-L49
So the simplest solution was to also drop v9.
Because v9 stoped receiving security patches I assumed no one would use it to power GraphQL servers.

So we can enable it back but we wouldn't be able to tests it.
I will try to remove our dependency on sane or revert it to the older version that supports v9.

},
"scripts": {
"watch": "babel-node ./resources/watch.js",
Expand Down