Skip to content

Commit

Permalink
Fix: workspace support in several commands (#4654)
Browse files Browse the repository at this point in the history
* Use lockfileFolder for CLI check

* Make "upgrade" work inside workspace packages

Executes "fetchRequestFromCwd" in actual cwd, which ensures
"outdated" and "upgrade" commands in workspace packages
operate on the correct dependencies and preserve unrelated lockfile
entries.

* Support workspaces in outdated and upgrade-interactive
  • Loading branch information
jgoz authored and arcanis committed Oct 9, 2017
1 parent daa599d commit 7323861
Show file tree
Hide file tree
Showing 18 changed files with 312 additions and 39 deletions.
27 changes: 27 additions & 0 deletions __tests__/commands/outdated.js
Original file line number Diff line number Diff line change
Expand Up @@ -130,3 +130,30 @@ test.concurrent('displays correct dependency types', (): Promise<void> => {
expect(body[2][4]).toBe('devDependencies');
});
});

test.concurrent('shows dependencies from entire workspace', async (): Promise<void> => {
await runOutdated([], {}, 'workspaces', (config, reporter, out): ?Promise<void> => {
const json: Object = JSON.parse(out);

expect(json.data.body).toHaveLength(4);
expect(json.data.body[0][0]).toBe('left-pad');
expect(json.data.body[0][1]).toBe('1.0.0');
expect(json.data.body[1][0]).toBe('left-pad');
expect(json.data.body[1][1]).toBe('1.0.1');
expect(json.data.body[2][0]).toBe('max-safe-integer');
expect(json.data.body[3][0]).toBe('right-pad');
});

const childFixture = {source: 'workspaces', cwd: 'child-a'};
return runOutdated([], {}, childFixture, (config, reporter, out): ?Promise<void> => {
const json: Object = JSON.parse(out);

expect(json.data.body).toHaveLength(4);
expect(json.data.body[0][0]).toBe('left-pad');
expect(json.data.body[0][1]).toBe('1.0.0');
expect(json.data.body[1][0]).toBe('left-pad');
expect(json.data.body[1][1]).toBe('1.0.1');
expect(json.data.body[2][0]).toBe('max-safe-integer');
expect(json.data.body[3][0]).toBe('right-pad');
});
});
43 changes: 43 additions & 0 deletions __tests__/commands/upgrade.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,3 +330,46 @@ test.concurrent('--latest works if there is an install script on a hoisted depen
'latest-with-install-script',
);
});

test.concurrent('upgrade to workspace root preserves child dependencies', (): Promise<void> => {
return runUpgrade(['[email protected]'], {latest: true}, 'workspaces', async (config): ?Promise<void> => {
const lockfile = explodeLockfile(await fs.readFile(path.join(config.cwd, 'yarn.lock')));

// child workspace deps
expect(lockfile.indexOf('[email protected]:')).toBeGreaterThanOrEqual(0);
expect(lockfile.indexOf('[email protected]:')).toBeGreaterThanOrEqual(0);
// root dep
expect(lockfile.indexOf('[email protected]:')).toBe(-1);
expect(lockfile.indexOf('[email protected]:')).toBeGreaterThanOrEqual(0);

const rootPkg = await fs.readJson(path.join(config.cwd, 'package.json'));
expect(rootPkg.devDependencies['max-safe-integer']).toEqual('1.0.1');

const childAPkg = await fs.readJson(path.join(config.cwd, 'child-a/package.json'));
const childBPkg = await fs.readJson(path.join(config.cwd, 'child-b/package.json'));
expect(childAPkg.dependencies['left-pad']).toEqual('1.0.0');
expect(childBPkg.dependencies['right-pad']).toEqual('1.0.0');
});
});

test.concurrent('upgrade to workspace child preserves root dependencies', (): Promise<void> => {
const fixture = {source: 'workspaces', cwd: 'child-a'};
return runUpgrade(['[email protected]'], {latest: true}, fixture, async (config): ?Promise<void> => {
const lockfile = explodeLockfile(await fs.readFile(path.join(config.lockfileFolder, 'yarn.lock')));

// untouched deps
expect(lockfile.indexOf('[email protected]:')).toBeGreaterThanOrEqual(0);
expect(lockfile.indexOf('[email protected]:')).toBeGreaterThanOrEqual(0);
// upgraded child workspace
expect(lockfile.indexOf('[email protected]:')).toBe(-1);
expect(lockfile.indexOf('[email protected]:')).toBeGreaterThanOrEqual(0);

const childAPkg = await fs.readJson(path.join(config.cwd, 'package.json'));
expect(childAPkg.dependencies['left-pad']).toEqual('1.1.0');

const rootPkg = await fs.readJson(path.join(config.lockfileFolder, 'package.json'));
const childBPkg = await fs.readJson(path.join(config.lockfileFolder, 'child-b/package.json'));
expect(rootPkg.devDependencies['max-safe-integer']).toEqual('1.0.0');
expect(childBPkg.dependencies['right-pad']).toEqual('1.0.0');
});
});
7 changes: 7 additions & 0 deletions __tests__/fixtures/outdated/workspaces/child-a/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "child-a",
"version": "1.0.0",
"dependencies": {
"max-safe-integer": "1.0.0"
}
}
8 changes: 8 additions & 0 deletions __tests__/fixtures/outdated/workspaces/child-b/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "child-b",
"version": "1.0.0",
"dependencies": {
"left-pad": "1.0.1",
"right-pad": "1.0.0"
}
}
11 changes: 11 additions & 0 deletions __tests__/fixtures/outdated/workspaces/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "my-project",
"private": true,
"devDependencies": {
"left-pad": "1.0.0"
},
"workspaces": [
"child-a",
"child-b"
]
}
19 changes: 19 additions & 0 deletions __tests__/fixtures/outdated/workspaces/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.0.0.tgz#c84e2417581bbb8eaf2b9e3d7a122e572ab1af37"

[email protected]:
version "1.0.1"
resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.0.1.tgz#d11b8e17e70e6ecb3b6bf2858fa99c40f819d13a"

[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/max-safe-integer/-/max-safe-integer-1.0.0.tgz#4662073a02c7e02d38153e25795489b20be6f01a"

[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/right-pad/-/right-pad-1.0.0.tgz#5ba6e56c0d7ec162d3626315c27a61f8aff42f15"
7 changes: 7 additions & 0 deletions __tests__/fixtures/upgrade/workspaces/child-a/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "child-a",
"version": "1.0.0",
"dependencies": {
"left-pad": "1.0.0"
}
}
7 changes: 7 additions & 0 deletions __tests__/fixtures/upgrade/workspaces/child-b/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"name": "child-b",
"version": "1.0.0",
"dependencies": {
"right-pad": "1.0.0"
}
}
11 changes: 11 additions & 0 deletions __tests__/fixtures/upgrade/workspaces/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "my-project",
"private": true,
"devDependencies": {
"max-safe-integer": "1.0.0"
},
"workspaces": [
"child-a",
"child-b"
]
}
15 changes: 15 additions & 0 deletions __tests__/fixtures/upgrade/workspaces/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1


[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/left-pad/-/left-pad-1.0.0.tgz#c84e2417581bbb8eaf2b9e3d7a122e572ab1af37"

[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/max-safe-integer/-/max-safe-integer-1.0.0.tgz#4662073a02c7e02d38153e25795489b20be6f01a"

[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/right-pad/-/right-pad-1.0.0.tgz#5ba6e56c0d7ec162d3626315c27a61f8aff42f15"
76 changes: 66 additions & 10 deletions src/cli/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ type Flags = {
exact: boolean,
tilde: boolean,
ignoreWorkspaceRootCheck: boolean,

// outdated, update-interactive
includeWorkspaceDeps: boolean,

// remove, upgrade
workspaceRootIsCwd: boolean,
};

/**
Expand Down Expand Up @@ -139,6 +145,12 @@ function normalizeFlags(config: Config, rawFlags: Object): Flags {
exact: !!rawFlags.exact,
tilde: !!rawFlags.tilde,
ignoreWorkspaceRootCheck: !!rawFlags.ignoreWorkspaceRootCheck,

// outdated, update-interactive
includeWorkspaceDeps: !!rawFlags.includeWorkspaceDeps,

// remove, update
workspaceRootIsCwd: rawFlags.workspaceRootIsCwd !== false,
};

if (config.getOption('ignore-scripts')) {
Expand Down Expand Up @@ -211,6 +223,13 @@ export class Install {
const usedPatterns = [];
let workspaceLayout;

// some commands should always run in the context of the entire workspace
const cwd =
this.flags.includeWorkspaceDeps || this.flags.workspaceRootIsCwd ? this.config.lockfileFolder : this.config.cwd;

// non-workspaces are always root, otherwise check for workspace root
const cwdIsRoot = !this.config.workspaceRootFolder || this.config.lockfileFolder === cwd;

// exclude package names that are in install args
const excludeNames = [];
for (const pattern of excludePatterns) {
Expand All @@ -224,17 +243,31 @@ export class Install {
excludeNames.push(parts.name);
}

const stripExcluded = (manifest: Manifest) => {
for (const exclude of excludeNames) {
if (manifest.dependencies && manifest.dependencies[exclude]) {
delete manifest.dependencies[exclude];
}
if (manifest.devDependencies && manifest.devDependencies[exclude]) {
delete manifest.devDependencies[exclude];
}
if (manifest.optionalDependencies && manifest.optionalDependencies[exclude]) {
delete manifest.optionalDependencies[exclude];
}
}
};

for (const registry of Object.keys(registries)) {
const {filename} = registries[registry];
const loc = path.join(this.config.lockfileFolder, filename);
const loc = path.join(cwd, filename);
if (!await fs.exists(loc)) {
continue;
}

this.rootManifestRegistries.push(registry);

const projectManifestJson = await this.config.readJson(loc);
await normalizeManifest(projectManifestJson, this.config.lockfileFolder, this.config, true);
await normalizeManifest(projectManifestJson, cwd, this.config, cwdIsRoot);

Object.assign(this.resolutions, projectManifestJson.resolutions);
Object.assign(manifest, projectManifestJson);
Expand Down Expand Up @@ -278,7 +311,7 @@ export class Install {

this.rootPatternsToOrigin[pattern] = depType;
patterns.push(pattern);
deps.push({pattern, registry, hint, optional});
deps.push({pattern, registry, hint, optional, workspaceName: manifest.name, workspaceLoc: manifest._loc});
}
};

Expand All @@ -287,27 +320,50 @@ export class Install {
pushDeps('optionalDependencies', projectManifestJson, {hint: 'optional', optional: true}, true);

if (this.config.workspaceRootFolder) {
const workspacesRoot = path.dirname(loc);
const workspaces = await this.config.resolveWorkspaces(workspacesRoot, projectManifestJson);
const workspaceLoc = cwdIsRoot ? loc : path.join(this.config.lockfileFolder, filename);
const workspacesRoot = path.dirname(workspaceLoc);

let workspaceManifestJson = projectManifestJson;
if (!cwdIsRoot) {
// the manifest we read before was a child workspace, so get the root
workspaceManifestJson = await this.config.readJson(workspaceLoc);
await normalizeManifest(workspaceManifestJson, workspacesRoot, this.config, true);
}

const workspaces = await this.config.resolveWorkspaces(workspacesRoot, workspaceManifestJson);
workspaceLayout = new WorkspaceLayout(workspaces, this.config);

// add virtual manifest that depends on all workspaces, this way package hoisters and resolvers will work fine
const workspaceDependencies = {...workspaceManifestJson.dependencies};
for (const workspaceName of Object.keys(workspaces)) {
const workspaceManifest = workspaces[workspaceName].manifest;
workspaceDependencies[workspaceName] = workspaceManifest.version;

// include dependencies from all workspaces
if (this.flags.includeWorkspaceDeps) {
pushDeps('dependencies', workspaceManifest, {hint: null, optional: false}, true);
pushDeps('devDependencies', workspaceManifest, {hint: 'dev', optional: false}, !this.config.production);
pushDeps('optionalDependencies', workspaceManifest, {hint: 'optional', optional: true}, true);
}
}
const virtualDependencyManifest: Manifest = {
_uid: '',
name: `workspace-aggregator-${uuid.v4()}`,
version: '1.0.0',
_registry: 'npm',
_loc: workspacesRoot,
dependencies: {},
dependencies: workspaceDependencies,
devDependencies: {...workspaceManifestJson.devDependencies},
optionalDependencies: {...workspaceManifestJson.optionalDependencies},
};
workspaceLayout.virtualManifestName = virtualDependencyManifest.name;
virtualDependencyManifest.dependencies = {};
for (const workspaceName of Object.keys(workspaces)) {
virtualDependencyManifest.dependencies[workspaceName] = workspaces[workspaceName].manifest.version;
}
const virtualDep = {};
virtualDep[virtualDependencyManifest.name] = virtualDependencyManifest.version;
workspaces[virtualDependencyManifest.name] = {loc: workspacesRoot, manifest: virtualDependencyManifest};

// ensure dependencies that should be excluded are stripped from the correct manifest
stripExcluded(cwdIsRoot ? virtualDependencyManifest : workspaces[projectManifestJson.name].manifest);

pushDeps('workspaces', {workspaces: virtualDep}, {hint: 'workspaces', optional: false}, true);
}

Expand Down
16 changes: 13 additions & 3 deletions src/cli/commands/outdated.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function hasWrapper(commander: Object, args: Array<string>): boolean {

export async function run(config: Config, reporter: Reporter, flags: Object, args: Array<string>): Promise<number> {
const lockfile = await Lockfile.fromDirectory(config.lockfileFolder);
const install = new Install(flags, config, reporter, lockfile);
const install = new Install({...flags, includeWorkspaceDeps: true}, config, reporter, lockfile);
let deps = await PackageRequest.getOutdatedPackages(lockfile, install, config, reporter);

if (args.length) {
Expand All @@ -33,18 +33,28 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg
const colorizeName = ({current, wanted, name}) => reporter.format[colorForVersions(current, wanted)](name);

if (deps.length) {
const usesWorkspaces = !!config.workspaceRootFolder;
const body = deps.map((info): Array<string> => {
return [
const row = [
colorizeName(info),
info.current,
colorizeDiff(info.current, info.wanted, reporter),
reporter.format.magenta(info.latest),
info.workspaceName || '',
getNameFromHint(info.hint),
reporter.format.cyan(info.url),
];
if (!usesWorkspaces) {
row.splice(4, 1);
}
return row;
});

reporter.table(['Package', 'Current', 'Wanted', 'Latest', 'Package Type', 'URL'], body);
const header = ['Package', 'Current', 'Wanted', 'Latest', 'Workspace', 'Package Type', 'URL'];
if (!usesWorkspaces) {
header.splice(4, 1);
}
reporter.table(header, body);
return 1;
}
return 0;
Expand Down
3 changes: 2 additions & 1 deletion src/cli/commands/remove.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export async function run(config: Config, reporter: Reporter, flags: Object, arg

// reinstall so we can get the updated lockfile
reporter.step(++step, totalSteps, reporter.lang('uninstallRegenerate'));
const reinstall = new Install({force: true, ...flags}, config, new NoopReporter(), lockfile);
const installFlags = {force: true, workspaceRootIsCwd: true, ...flags};
const reinstall = new Install(installFlags, config, new NoopReporter(), lockfile);
await reinstall.init();

//
Expand Down
Loading

0 comments on commit 7323861

Please sign in to comment.