-
Notifications
You must be signed in to change notification settings - Fork 27.4k
chore(*): update minimum Yarn version, do some Yarn-related cleanups #16714
Conversation
Yarn 1.10 added the integrity field to the lockfile, making newer Yarn users have their lockfile changed a lot if they run `yarn`. This commit updates the required Yarn version to be at least 1.10.1 and changes Travis & Jenkins to use Yarn 1.10.1
The grunt field suggested it's the grunt package version we're checking while we check the grunt-cli version instead.
… " -- " The " -- " separator is necessary in npm but not in Yarn. In fact, it's deprecated in Yarn and some future version is supposed to start passing this parameter directly to the scripts which may break them.
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.
🎉
package.json
Outdated
@@ -10,8 +10,8 @@ | |||
}, | |||
"engines": { | |||
"node": "^8.9.1", |
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.
Can we change the node engine to be more lenient to newer versions of node too?
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.
Sure! Do we want to upgrade to Node 10 or stay at 8, just newer?
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 think we should make it relaxed enough to accept any engines that work with our build. So perhaps >= 8.9.1 < 11
?
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.
@petebacondarwin We don't have a way to verify if it keeps working at a version we don't work on. Also, .nvmrc
needs to specify one version, we can't use 2 at the same time. So if we want to allow newer versions maybe it'd be better to say >=8.12.0
so that all newer versions are accepted?
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.
Happy to go with >=8.12.0
@@ -21,21 +21,21 @@ rm -f angular.js.size | |||
|
|||
|
|||
# BUILD # | |||
yarn run grunt -- ci-checks package --no-color |
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.
❤️
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.
That reminds me I can shorten it to yarn grunt
instead of yarn run grunt
... We already use the former form in a few places.
It's enough to use `yarn grunt` instead of `grunt` and the global grunt-cli installation is no longer needed.
The former form is shorter.
As opposed to npm, `yarn binName` invokes a binary named `binName` exposed by the respective package so the `grant` Yarn script is no longer needed.
Included changes: *Update minimum Yarn version from 1.3.2 to 1.10.1* Yarn 1.10 added the integrity field to the lockfile, making newer Yarn users have their lockfile changed a lot if they run `yarn`. This commit updates the required Yarn version to be at least 1.10.1 and changes Travis & Jenkins to use Yarn 1.10.1 *Change the package.json's engines grunt field to grunt-cli* The grunt field suggested it's the grunt package version we're checking while we check the grunt-cli version instead. *Stop separating Yarn script arguments from script names via " -- "* The " -- " separator is necessary in npm but not in Yarn. In fact, it's deprecated in Yarn and some future version is supposed to start passing this parameter directly to the scripts which may break them. *Don't install grunt-cli globally during the build* It's enough to use `yarn grunt` instead of `grunt` and the global grunt-cli installation is no longer needed. *Use `yarn grunt` instead of `yarn run grunt`* The former form is shorter. *Don't define the `grunt` Yarn script* As opposed to npm, `yarn binName` invokes a binary named `binName` exposed by the respective package so the `grant` Yarn script is no longer needed. *Allow Node versions newer than 8; bump the minimum* Closes #16714
Just a note: when running yarn, I still get |
@Narretz we parse the |
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Build change.
What is the current behavior? (You can also link to an open issue here)
See the next section.
What is the new behavior (if this is a feature change)?
Included changes:
chore(*): update minimum Yarn version from 1.3.2 to 1.10.1
Yarn 1.10 added the integrity field to the lockfile, making newer Yarn users
have their lockfile changed a lot if they run
yarn
. This commit updates therequired Yarn version to be at least 1.10.1 and changes Travis & Jenkins to use
Yarn 1.10.1
chore(*): change the package.json's engines grunt field to grunt-cli
The grunt field suggested it's the grunt package version we're checking while
we check the grunt-cli version instead.
chore(*): stop separating Yarn script arguments from script names via " -- "
The " -- " separator is necessary in npm but not in Yarn. In fact, it's
deprecated in Yarn and some future version is supposed to start passing this
parameter directly to the scripts which may break them.
chore(*): Don't install grunt-cli globally during the build
It's enough to use
yarn grunt
instead ofgrunt
and the global grunt-cliinstallation is no longer needed.
Does this PR introduce a breaking change?
No.
Please check if the PR fulfills these requirements
Fix/Feature: Docs have been added/updatedFix/Feature: Tests have been added; existing tests passOther information: