-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade CLI dependencies #3743
Upgrade CLI dependencies #3743
Conversation
@moroine Amazing! 馃帀 Let me check in with folks to make sure the bump to node >= 10 is acceptable. |
Also there's a few CLI tests that are still failing, and they can be reproduced locally with running |
cli/src/commands/install.js
Outdated
flowVersion: { | ||
alias: 'f', | ||
describe: | ||
'The Flow version that fetched libdefs must be compatible ' + 'with', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it was already there but wondering about the concatenation... Can't see any use.
cli/src/lib/npm/npmLibDefs.js
Outdated
@@ -70,11 +70,11 @@ async function extractLibDefsFromNpmPkgDir( | |||
const pkgDirItems = await fs.readdir(pkgDirPath); | |||
|
|||
if (validating) { | |||
const fullPkgName = `${scope === null ? '' : scope + '/'}${pkgName}`; | |||
const fullPkgName = `${scope === null ? '' : scope + '/'}${pkgName}popopko`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to remove this? :)
cli/src/lib/npm/npmLibDefs.js
Outdated
@@ -88,8 +88,9 @@ async function extractLibDefsFromNpmPkgDir( | |||
}) | |||
.then() | |||
.catch(error => { | |||
console.log(error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove?
@moroine Really nice, thanks! About the node >= 10 bump, it's okay if we release this as a major version I guess. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
馃憦 馃憦 馃憦
@@ -13,48 +13,49 @@ | |||
"version": "2.6.2", | |||
"main": "dist/cli.js", | |||
"bin": "dist/cli.js", | |||
"engines": { | |||
"node": ">=10.0.0" | |||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good timing on this change (now that Node 8 is no longer maintained)
@AndrewSouthpaw test failing in NodeJS 12 is due to change in NodeJS 11 when switching to a stable algorithm. I've made the fix to have a deterministic order (ignoring prerelease). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
馃
Great work @moroine ! Merging. |
@gantoine You'll probably need to bump the flow-typed version. |
@AndrewSouthpaw On it, I'll get one out tonight. |
Minor deps upgrades:
@octokit/rest
:16.33.1
~>16.43.1
colors
:1.3.2
~>1.4.0
flowgen
:1.9.0
~>1.10.0
glob
:7.1.3
~>7.1.3
prettier
:1.18.2
~>1.19.1
table
:5.0.2
~>5.4.6
Major deps upgrades:
fs-extra
:7.0.0
~>8.1.0
got
:8.3.2
~>10.5.7
mkdirp
:0.5.1
~>1.0.3
rimraf
:2.6.2
~>3.0.2
semver
:5.5.1
~>7.1.3
unzipper
:0.9.3
~>0.10.8
which
:1.3.1
~>2.0.2
yargs
:12.0.2
~>15.1.0
Removed deps:
@babel/polyfill
: not needed any more thanks to dropping end-of-life Node versionsDev-deps upgrades:
@babel/cli
:7.1.0
~>7.8.4
@babel/core
:7.1.0
~>7.8.4
@babel/preset-env
:7.1.0
~>7.8.4
@babel/preset-flow
:7.1.0
~>7.8.4
babel-core
: removedbabel-eslint
:10.0.1
~>10.0.3
babel-jest
:23.6.0
~>25.1.0
eslint
:5.6.0
~>6.8.0
eslint-plugin-flowtype
:2.50.3
~>4.6.0
eslint-plugin-prettier
:2.7.0
~>3.1.2
flow-bin
:0.81.0
~>0.118.0
馃殌jest
:23.6.0
~>25.1.0
CI:
exit 0
to return error code on failed testMisc:
@octokit/rest
deprecation messageFlow Defs:
mkdirp_v1.0.x
will need to be added later in definitions (will do a separated PR)yargs_v15.0.x
has been created by copy/pasteyargs_v12.0.x
this should be alright as API didn't change much but would require a proper PR.Breaking Change: