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

chore: Update nvmrc with latest lts #16610

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Aug 29, 2019

On master:

$ nvm use
Now using node v8.4.0 (npm v5.3.0)
$ yarn
error gitlab@10.2.1: The engine "node" is incompatible with this module. Expected version ">=10.0.0". Got "8.4.0"
error Found incompatible module.

Updated with latest node lts release.

@sizebot
Copy link

sizebot commented Aug 29, 2019

No significant bundle size changes to report.

Generated by 🚫 dangerJS against a5d3097

@threepointone
Copy link
Contributor

This seems reasonable, but could you help me understand what the gitlab connection is? Is it some deeper dependency we have? I didn't find it in yarn.lock, so I'm confused.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Aug 29, 2019

TL;DR: It's a dependency of danger which was only recently bumped to 9. It probably got introduced during that PR

$ yarn why gitlab
yarn why v1.17.3
[1/4] Why do we have the module "gitlab"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "gitlab@10.2.1"
info Reasons this module exists
   - "_project_#danger" depends on it
   - Hoisted from "_project_#danger#gitlab"
info Disk size without dependencies: "812KB"
info Disk size with unique dependencies: "1.15MB"
info Disk size with transitive dependencies: "2.69MB"
info Number of shared dependencies: 18
Done in 1.73s.

@eps1lon
Copy link
Collaborator Author

eps1lon commented Aug 29, 2019

@bvaughn
Copy link
Contributor

bvaughn commented Aug 29, 2019

I don't have any objections to updating the default Node. I usually use v11 locally.

If our main concern is the warning being printed to the console, we could also use the --ignore-engines flag to suppress it. But we might as well just update from my POV.

@threepointone
Copy link
Contributor

Alright, let's get this in. Thanks!

(We also agree to blame @bvaughn if it breaks stuff. Don't tell him.)

@threepointone threepointone merged commit 16329bd into facebook:master Aug 29, 2019
@eps1lon eps1lon deleted the chore/update-nvmrc branch August 29, 2019 16:01
@eps1lon
Copy link
Collaborator Author

eps1lon commented Aug 29, 2019

Just for posterity: node 8 reaches end of life in december, 10 is active LTS and 11 is no longer maintained (odd version numbers have a short lifespan, even numbers are LTS candidates). Full node release cycle

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants