Skip to content

Commit 39d6fe2

Browse files
authored
Fix: Fix numeric versions resolving to local files (#4088)
**Summary** Fixes #4013. We were blindly priotizing local files (even if they are not directories) if there's anything on the file system matching a pattern for a package. This patch ensures that we only do this if the pattern is not a valid semver range and the matched local entity is a directory. **Test plan** Added a new unit test and modified an incorrect old one. Also manually verified that the issue described in #4013 does not happen anymore.
1 parent 280b6eb commit 39d6fe2

File tree

11 files changed

+68
-11
lines changed

11 files changed

+68
-11
lines changed
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`install-file-as-default-no-semver 1`] = `
4+
"{
5+
\\"author\\": \\"AJ ONeal <[email protected]> (http://coolaj86.info)\\",
6+
\\"name\\": \\"foo\\",
7+
\\"description\\": \\"A test module with no \`main\`, \`lib\`, or \`dependencies\` specified\\",
8+
\\"version\\": \\"1.0.0\\",
9+
\\"repository\\": {
10+
\\"type\\": \\"git\\",
11+
\\"url\\": \\"git://github.com/coolaj86/node-pakman.git\\"
12+
},
13+
\\"engines\\": {
14+
\\"node\\": \\">= v0.2\\"
15+
}
16+
}
17+
"
18+
`;

__tests__/commands/install/integration.js

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ test.concurrent('root install with optional deps', (): Promise<void> => {
385385
});
386386

387387
test.concurrent('install file: protocol with relative paths', (): Promise<void> => {
388-
return runInstall({noLockfile: true}, 'install-file-relative', async config => {
388+
return runInstall({}, 'install-file-relative', async config => {
389389
expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'root-a', 'index.js'))).toEqual('foobar;\n');
390390
});
391391
});
@@ -459,17 +459,30 @@ test.concurrent('install file: link file dependencies', async (): Promise<void>
459459
});
460460

461461
test.concurrent('install file: protocol', (): Promise<void> => {
462-
return runInstall({noLockfile: true}, 'install-file', async config => {
462+
return runInstall({lockfile: false}, 'install-file', async config => {
463463
expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'foo', 'index.js'))).toEqual('foobar;\n');
464464
});
465465
});
466466

467467
test.concurrent('install with file: protocol as default', (): Promise<void> => {
468-
return runInstall({noLockfile: true}, 'install-file-as-default', async config => {
468+
return runInstall({}, 'install-file-as-default', async config => {
469469
expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'foo', 'index.js'))).toEqual('foobar;\n');
470470
});
471471
});
472472

473+
test.concurrent("don't install with file: protocol as default if target is a file", (): Promise<void> => {
474+
// $FlowFixMe
475+
return expect(runInstall({lockfile: false}, 'install-file-as-default-no-file')).rejects.toBeDefined();
476+
});
477+
478+
test.concurrent("don't install with file: protocol as default if target is valid semver", (): Promise<void> => {
479+
return runInstall({}, 'install-file-as-default-no-semver', async config => {
480+
expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'foo', 'package.json'))).toMatchSnapshot(
481+
'install-file-as-default-no-semver',
482+
);
483+
});
484+
});
485+
473486
// When local packages are installed, dependencies with different forms of the same relative path
474487
// should be deduped e.g. 'file:b' and 'file:./b'
475488
test.concurrent('install file: dedupe dependencies 1', (): Promise<void> => {
@@ -505,7 +518,7 @@ test.concurrent('install file: dedupe dependencies 3', (): Promise<void> => {
505518
});
506519

507520
test.concurrent('install everything when flat is enabled', (): Promise<void> => {
508-
return runInstall({noLockfile: true, flat: true}, 'install-file', async config => {
521+
return runInstall({lockfile: false, flat: true}, 'install-file', async config => {
509522
expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'foo', 'index.js'))).toEqual('foobar;\n');
510523
});
511524
});
@@ -587,7 +600,7 @@ test.concurrent('install should run install scripts in the order of dependencies
587600
});
588601

589602
test.concurrent('install with comments in manifest', (): Promise<void> => {
590-
return runInstall({noLockfile: true}, 'install-with-comments', async config => {
603+
return runInstall({lockfile: false}, 'install-with-comments', async config => {
591604
expect(await fs.readFile(path.join(config.cwd, 'node_modules', 'foo', 'index.js'))).toEqual('foobar;\n');
592605
});
593606
});
@@ -759,7 +772,7 @@ test.concurrent('offline mirror can be disabled locally', (): Promise<void> => {
759772

760773
// sync test because we need to get all the requests to confirm their validity
761774
test('install a scoped module from authed private registry', (): Promise<void> => {
762-
return runInstall({noLockfile: true}, 'install-from-authed-private-registry', async config => {
775+
return runInstall({}, 'install-from-authed-private-registry', async config => {
763776
const authedRequests = request.__getAuthedRequests();
764777

765778
expect(authedRequests[0].url).toEqual('https://registry.yarnpkg.com/@types%2flodash');
@@ -774,7 +787,7 @@ test('install a scoped module from authed private registry', (): Promise<void> =
774787
});
775788

776789
test('install a scoped module from authed private registry with a missing trailing slash', (): Promise<void> => {
777-
return runInstall({noLockfile: true}, 'install-from-authed-private-registry-no-slash', async config => {
790+
return runInstall({}, 'install-from-authed-private-registry-no-slash', async config => {
778791
const authedRequests = request.__getAuthedRequests();
779792

780793
expect(authedRequests[0].url).toEqual('https://registry.yarnpkg.com/@types%2flodash');

__tests__/fixtures/install/install-file-as-default-no-file/bar

Whitespace-only changes.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"dependencies": {
3+
"foo": "bar"
4+
}
5+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
foobar;
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"name": "1",
3+
"version": "0.0.0",
4+
"main": "index.js"
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"dependencies": {
3+
"foo": "1"
4+
}
5+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
2+
# yarn lockfile v1
3+
"foo@file:bar":
4+
version "0.0.0"
Binary file not shown.
Binary file not shown.

0 commit comments

Comments
 (0)