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

fix(cli): fail when yarn adding Github package with no version (#5292) #5318

Merged
merged 2 commits into from
Feb 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions __tests__/commands/add.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ test.concurrent('install from github', async () => {
await runAdd(['substack/node-mkdirp#master'], {}, 'install-github');
});

test.concurrent('install from github with invalid version should fail', async () => {
let message = '';
try {
await runAdd(['yarnpkg/example-yarn-package#invalid-package-json-version'], {}, 'install-github');
} catch (err) {
message = err.message;
}
expect(message).toEqual(expect.stringContaining('invalid package version'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would be preferable to use .rejects and assert on the error message using toThrow or the like, but it looks like this is still problematic: jestjs/jest#3601

});

test.concurrent('install with --dev flag', async () => {
await runAdd(['[email protected]'], {dev: true}, 'add-with-flag', async config => {
const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));
Expand Down
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
HTTP/1.1 200 OK
Content-Security-Policy: default-src 'none'; style-src 'unsafe-inline'; sandbox
Strict-Transport-Security: max-age=31536000
X-Content-Type-Options: nosniff
X-Frame-Options: deny
X-XSS-Protection: 1; mode=block
ETag: "d5561550f2f68bb15a0801ccab1c6f5362959c40"
Content-Type: text/plain; charset=utf-8
Cache-Control: max-age=300
X-Geo-Block-List:
X-GitHub-Request-Id: 49EE:29366:1920CD4:1A583B0:5A779863
Content-Length: 463
Accept-Ranges: bytes
Date: Sun, 04 Feb 2018 23:33:57 GMT
Via: 1.1 varnish
Connection: keep-alive
X-Served-By: cache-pao17421-PAO
X-Cache: MISS
X-Cache-Hits: 0
X-Timer: S1517787238.609785,VS0,VE119
Vary: Authorization,Accept-Encoding
Access-Control-Allow-Origin: *
X-Fastly-Request-ID: b02c8764f6d89b30ee060159193da00b51b9fbd8
Expires: Sun, 04 Feb 2018 23:38:57 GMT
Source-Age: 0

{
"name": "example-yarn-package",
"description": "An example package to demonstrate Yarn",
"main": "index.js",
"repository": {
"url": "github.com/yarnpkg/example-yarn-package",
"type": "git"
},
"scripts": {
"test": "jest"
},
"author": "Yarn Contributors",
"license": "BSD-2-Clause",
"dependencies": {
"lodash": "^4.16.2"
},
"devDependencies": {
"jest-cli": "15.1.1"
},
"jest": {
"testEnvironment": "node"
}
}
4 changes: 4 additions & 0 deletions src/package-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@ export default class PackageRequest {
// find version info for this package pattern
const info: Manifest = await this.findVersionInfo();

if (!semver.valid(info.version)) {
throw new MessageError(this.reporter.lang('invalidPackageVersion', info.name, info.version));
}

info.fresh = fresh;
cleanDependencies(info, false, this.reporter, () => {
// swallow warnings
Expand Down
1 change: 1 addition & 0 deletions src/reporters/lang/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ const messages = {
invalidHostedGitFragment: 'Invalid hosted git fragment $0.',
invalidFragment: 'Invalid fragment $0.',
invalidPackageName: 'Invalid package name.',
invalidPackageVersion: "Can't add $0: invalid package version $1.",
couldntFindManifestIn: "Couldn't find manifest in $0.",
shrinkwrapWarning:
'npm-shrinkwrap.json found. This will not be updated or respected. See https://yarnpkg.com/en/docs/migrating-from-npm for more information.',
Expand Down
2 changes: 1 addition & 1 deletion src/util/git.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ export default class Git implements GitRefResolvingInterface {
});
if (!resolvedResult) {
throw new MessageError(
this.reporter.lang('couldntFindMatch', version, Object.keys(refs).join(','), this.gitUrl.repository),
this.reporter.lang('couldntFindMatch', version, Array.from(refs.keys()).join(','), this.gitUrl.repository),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

refs is a Map which always returns an empty array with Object.keys()

);
}

Expand Down