Skip to content

Commit

Permalink
Fix for #1214
Browse files Browse the repository at this point in the history
Scoped dependencies are nested one folder deeper than traditional
dependencies, the possiblyExtraneous needed updating to work with the
actual dependencies, not the entire scoped folder.
  • Loading branch information
tgriesser committed Dec 6, 2016
1 parent eb66b9c commit 546627e
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 5 deletions.
18 changes: 18 additions & 0 deletions __tests__/commands/install/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -681,3 +681,21 @@ test.concurrent('install a module with incompatible optional dependency should s
assert.ok(!(await fs.exists(path.join(config.cwd, 'node_modules', 'dep-a'))));
});
});

test.concurrent('install will not overwrite files in symlinked scoped directories', async (): Promise<void> => {
await runInstall({}, 'install-dont-overwrite-linked-scoped', async (config): Promise<void> => {
const dependencyPath = path.join(config.cwd, 'node_modules', '@fakescope', 'fake-dependency');
assert.equal(
'Symlinked scoped package test',
(await fs.readJson(path.join(dependencyPath, 'package.json'))).description,
);
assert.ok(!(await fs.exists(path.join(dependencyPath, 'index.js'))));
}, async (cwd) => {
const dirToLink = path.join(cwd, 'dir-to-link');
await fs.mkdirp(path.join(cwd, '.yarn-link', '@fakescope'));
await fs.symlink(dirToLink, path.join(cwd, '.yarn-link', '@fakescope', 'fake-dependency'));
await fs.mkdirp(path.join(cwd, 'node_modules', '@fakescope'));
await fs.symlink(dirToLink, path.join(cwd, 'node_modules', '@fakescope', 'fake-dependency'));
});
});

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
yarn-offline-mirror=./mirror-for-offline
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "@fakescope/fake-dependency",
"description": "Symlinked scoped package test",
"version": "1.0.1",
"dependencies": {},
"license": "MIT"
}
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"@fakescope/fake-dependency": "1.0.1"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1
"@fakescope/fake-dependency@1.0.1":
version "1.0.1"
resolved "@fakescope-fake-dependency-1.0.1.tgz#477dafd486d856af0b3faf5a5f1c895001221609"
22 changes: 18 additions & 4 deletions src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,21 @@ export default class Config {

await fs.mkdirp(this.globalFolder);
await fs.mkdirp(this.linkFolder);
this.linkedModules = await fs.readdir(this.linkFolder);

this.linkedModules = [];

const linkedModules = await fs.readdir(this.linkFolder);

for (const dir of linkedModules) {
const linkedPath = path.join(this.linkFolder, dir);

if (dir[0] === '@') { // it's a scope, not a package
const scopedLinked = await fs.readdir(linkedPath);
this.linkedModules.push(...scopedLinked.map((scopedDir) => path.join(dir, scopedDir)));
} else {
this.linkedModules.push(dir);
}
}

for (const key of Object.keys(registries)) {
const Registry = registries[key];
Expand Down Expand Up @@ -372,7 +386,7 @@ export default class Config {

/**
* Read normalized package info according yarn-metadata.json
* throw an error if package.json was not found
* throw an error if package.json was not found
*/

async readManifest(dir: string, priorityRegistry?: RegistryNames, isRoot?: boolean = false): Promise<Manifest> {
Expand All @@ -386,8 +400,8 @@ export default class Config {
}

/**
* try get the manifest file by looking
* 1. mainfest fiel in cache
* try get the manifest file by looking
* 1. mainfest fiel in cache
* 2. manifest file in registry
*/
maybeReadManifest(dir: string, priorityRegistry?: RegistryNames, isRoot?: boolean = false): Promise<?Manifest> {
Expand Down
15 changes: 14 additions & 1 deletion src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ export default class PackageLinker {
});
}

// keep track of all scoped paths to remove empty scopes after copy
const scopedPaths = new Set();

// register root & scoped packages as being possibly extraneous
const possibleExtraneous: Set<string> = new Set();
for (const folder of this.config.registryFolders) {
Expand All @@ -158,12 +161,14 @@ export default class PackageLinker {
let filepath;
for (const file of files) {
filepath = path.join(loc, file);
possibleExtraneous.add(filepath);
if (file[0] === '@') { // it's a scope, not a package
scopedPaths.add(filepath);
const subfiles = await fs.readdir(filepath);
for (const subfile of subfiles) {
possibleExtraneous.add(path.join(filepath, subfile));
}
} else {
possibleExtraneous.add(filepath);
}
}
}
Expand Down Expand Up @@ -200,6 +205,14 @@ export default class PackageLinker {
},
});

// remove any empty scoped directories
for (const scopedPath of scopedPaths) {
const modules = await fs.readdir(scopedPath);
if (modules.length === 0) {
await fs.unlink(scopedPath);
}
}

//
if (this.config.binLinks) {
const tickBin = this.reporter.progress(flatTree.length);
Expand Down

0 comments on commit 546627e

Please sign in to comment.