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

Update: warn when used with not supported Node versions #4502

Merged
merged 2 commits into from
Sep 22, 2017
Merged

Conversation

BYK
Copy link
Member

@BYK BYK commented Sep 19, 2017

Summary

Fixes #4501, refs #4490, refs #4284. Yarn now warns when it
detects it is running in a Node version that is not fully
supported and warns the user about this. This is different than
the hard Node 4+ check in the entry file since in that case,
Yarn wouldn't run at all due to syntax incompatibilities. This
warning is to signal that users may encounter unexpected errors
but are allowed to use Yarn if they wish. It also adds a new
flag to suppress this warning: --no-node-version-check.

Test plan

Since we cannot add unsupported Node versions to our CI and
spoof the Node version internally, this has to be tested
manually, which I did.

**Summary**

Fixes #4501, refs #4490, refs #4284. Yarn now warns when it
detects it is running in a Node version that is not fully
supported and warns the user about this. This is different than
the hard Node 4+ check in the entry file since in that case,
Yarn wuoldn't run at all due to syntax incompatibilities. This
warning is to signal that users may encounter unexpected errors
but are allowed to use Yarn if they wish. It also adds a new
flag to suppress this warning: `--no-node-version-check`.

**Test plan**

Since we cannot add unsupported Node versions to our CI and
spoof the Node version internally, this has to be tested
manually, which I did.
@BYK BYK requested a review from arcanis September 19, 2017 12:32
@buildsize
Copy link

buildsize bot commented Sep 19, 2017

This change will increase the build size from 9.7 MB to 9.71 MB, an increase of 10.74 KB (0%)

File name Previous Size New Size Change
yarn-[version].noarch.rpm 835.35 KB 835.92 KB 588 bytes (0%)
yarn-[version].js 3.7 MB 3.7 MB 4.69 KB (0%)
yarn-legacy-[version].js 3.75 MB 3.75 MB 4.69 KB (0%)
yarn-v[version].tar.gz 839.7 KB 840.24 KB 553 bytes (0%)
yarn_[version]all.deb 635.46 KB 635.71 KB 258 bytes (0%)

@SimenB
Copy link
Contributor

SimenB commented Sep 20, 2017

@BYK
Copy link
Member Author

BYK commented Sep 20, 2017

@SimenB yes but that requires us to run Yarn from inside the same process that mocks the version which is not easy or may not be possible with the CLI entry point.

@@ -98,6 +100,7 @@ export function main({
'prepend the node executable dir to the PATH in scripts',
boolify,
);
commander.option('--no-node-version-check', 'do not warn when using a potentially unsupported Node version');
Copy link
Member

Choose a reason for hiding this comment

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

That's a long command name :D

Copy link
Member

Choose a reason for hiding this comment

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

--no-node-version-check-or-else-youll-be-fired

@BYK BYK merged commit 2c2b23e into master Sep 22, 2017
@BYK BYK deleted the warn-versions branch September 22, 2017 11:58
@mkj28
Copy link

mkj28 commented Sep 28, 2017

Trying to understand why was node 7 dropped from support? Cannot find the reason in the documents linked here.

@BYK
Copy link
Member Author

BYK commented Sep 29, 2017

@mkj28 I think I dropped it unintentionally since it is not listed anywhere and it is not LTS. We do not test anything with Node 7 so we do not commit to supporting it. This doesn't mean it is going to break.

joaolucasl pushed a commit to joaolucasl/yarn that referenced this pull request Oct 27, 2017
**Summary**

Fixes yarnpkg#4501, refs yarnpkg#4490, refs yarnpkg#4284. Yarn now warns when it
detects it is running in a Node version that is not fully
supported and warns the user about this. This is different than
the hard Node 4+ check in the entry file since in that case,
Yarn wouldn't run at all due to syntax incompatibilities. This
warning is to signal that users may encounter unexpected errors
but are allowed to use Yarn if they wish. It also adds a new
flag to suppress this warning: `--no-node-version-check`.

**Test plan**

Since we cannot add unsupported Node versions to our CI and
spoof the Node version internally, this has to be tested
manually, which I did.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants