Skip to content

Commit

Permalink
Pass npmClientArgs to yarn workspaces install command (#1041)
Browse files Browse the repository at this point in the history 
Refactor bootstrap to use same code path for all installs (eventually) and move internal npmGlobalStyle parameter into options property.

Fixes #1032
  • Loading branch information
@evocateur
evocateur committed Oct 2, 2017
1 parent 7aa16e5 commit a132b6e
Show file tree
Hide file tree
Showing 4 changed files with 179 additions and 111 deletions.
102 changes: 41 additions & 61 deletions src/NpmUtilities.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,43 @@ function splitVersion(dep) {
return dep.match(/^(@?[^@]+)(?:@(.+))?/).slice(1, 3);
}

function execInstall(directory, {
registry,
npmClient,
npmClientArgs,
npmGlobalStyle,
mutex,
}) {
// build command, arguments, and options
const opts = NpmUtilities.getExecOpts(directory, registry);
const args = ["install"];
let cmd = npmClient || "npm";

if (npmGlobalStyle) {
cmd = "npm";
args.push("--global-style");
}

if (cmd === "yarn" && mutex) {
args.push("--mutex", mutex);
}

if (cmd === "yarn") {
args.push("--non-interactive");
}

if (npmClientArgs && npmClientArgs.length) {
args.push(...npmClientArgs);
}

log.silly("installInDir", [cmd, args]);
return ChildProcessUtilities.exec(cmd, args, opts);
}

export default class NpmUtilities {
static installInDir(directory, dependencies, config, npmGlobalStyle, callback) {
static installInDir(directory, dependencies, config, callback) {
log.silly("installInDir", path.basename(directory), dependencies);

// npmGlobalStyle is an optional argument
if (typeof npmGlobalStyle === "function") {
callback = npmGlobalStyle;
npmGlobalStyle = false;
}

// Nothing to do if we weren't given any deps.
if (!(dependencies && dependencies.length)) {
log.verbose("installInDir", "no dependencies to install");
Expand Down Expand Up @@ -67,64 +94,17 @@ export default class NpmUtilities {

log.silly("installInDir", "writing tempJson", tempJson);
// Write out our temporary cooked up package.json and then install.
writePkg(packageJson, tempJson).then(() => {
// build command, arguments, and options
const opts = NpmUtilities.getExecOpts(directory, config.registry);
const args = ["install"];
let cmd = config.npmClient || "npm";

if (npmGlobalStyle) {
cmd = "npm";
args.push("--global-style");
}

if (cmd === "yarn" && config.mutex) {
args.push("--mutex", config.mutex);
}

if (cmd === "yarn") {
args.push("--non-interactive");
}

if (config.npmClientArgs && config.npmClientArgs.length) {
args.push(...config.npmClientArgs);
}

log.silly("installInDir", [cmd, args]);
ChildProcessUtilities.exec(cmd, args, opts, done);
}).catch(done);
writePkg(packageJson, tempJson)
.then(() => execInstall(directory, config))
.then(() => done(), done);
});
}

static installInDirOriginalPackageJson(directory, config, npmGlobalStyle, callback) {
log.silly("installInDir", path.basename(directory));

// npmGlobalStyle is an optional argument
if (typeof npmGlobalStyle === "function") {
callback = npmGlobalStyle;
npmGlobalStyle = false;
}

const packageJson = path.join(directory, "package.json");

log.silly("installInDir", packageJson);

// build command, arguments, and options
const opts = NpmUtilities.getExecOpts(directory, config.registry);
const args = ["install"];
let cmd = config.npmClient || "npm";

if (npmGlobalStyle) {
cmd = "npm";
args.push("--global-style");
}

if (cmd === "yarn" && config.mutex) {
args.push("--mutex", config.mutex);
}
static installInDirOriginalPackageJson(directory, config, callback) {
log.silly("installInDirOriginalPackageJson", directory);

log.silly("installInDir", [cmd, args]);
ChildProcessUtilities.exec(cmd, args, opts, callback);
return execInstall(directory, config)
.then(() => callback(), callback);
}


Expand Down
12 changes: 7 additions & 5 deletions src/commands/BootstrapCommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,16 @@ export default class BootstrapCommand extends Command {
}

initialize(callback) {
const { registry, npmClient, npmClientArgs, mutex } = this.options;
const { registry, npmClient, npmClientArgs, mutex, hoist } = this.options;

// Use `npm install --global-style` for leaves when hoisting is enabled
const npmGlobalStyle = !!hoist;

this.npmConfig = {
registry,
npmClient,
npmClientArgs,
npmGlobalStyle,
mutex
};

Expand All @@ -68,10 +72,12 @@ export default class BootstrapCommand extends Command {
if (npmClient === "yarn" && !mutex) {
return getPort({ port: 42424, host: '0.0.0.0' }).then((port) => {
this.npmConfig.mutex = `network:${port}`;
this.logger.silly("npmConfig", this.npmConfig);
callback(null, true);
}).catch(callback);
}

this.logger.silly("npmConfig", this.npmConfig);
callback(null, true);
}

Expand Down Expand Up @@ -113,7 +119,6 @@ export default class BootstrapCommand extends Command {
(cb) => this.preparePackages(cb)
], callback);
}

}

installRootPackageOnly(callback) {
Expand Down Expand Up @@ -476,8 +481,6 @@ export default class BootstrapCommand extends Command {
}

// Install anything that needs to go into the leaves.
// Use `npm install --global-style` for leaves when hoisting is enabled
const npmGlobalStyle = this.options.hoist;
Object.keys(leaves)
.map((pkgName) => ({ pkg: this.packageGraph.get(pkgName).package, deps: leaves[pkgName] }))
.forEach(({ pkg, deps }) => {
Expand All @@ -489,7 +492,6 @@ export default class BootstrapCommand extends Command {
pkg.location,
deps.map(({ dependency }) => dependency),
this.npmConfig,
npmGlobalStyle,
(err) => {
tracker.verbose("installed leaf", pkg.name);
tracker.completeWork(1);
Expand Down
52 changes: 34 additions & 18 deletions test/BootstrapCommand.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,13 @@ describe("BootstrapCommand", () => {
expect(installedPackagesInDirectories(testDir)).toMatchSnapshot();
expect(removedDirectories(testDir)).toMatchSnapshot();

// foo@^1.0.0 will be hoisted, and should not use global style
expect(NpmUtilities.installInDir).toBeCalledWith(
expect.any(String),
expect.arrayContaining(["foo@^1.0.0"]),
expect.any(Object),
expect.any(Function)
);

// foo@0.1.2 differs from the more common foo@^1.0.0
expect(NpmUtilities.installInDir).lastCalledWith(
expect.stringContaining("package-3"),
expect.arrayContaining(["foo@0.1.12"]),
expect.any(Object),
"**", // npmGlobalStyle
expect.objectContaining({
npmGlobalStyle: true,
}),
expect.any(Function)
);
});
Expand Down Expand Up @@ -181,6 +174,19 @@ describe("BootstrapCommand", () => {
expect(installedPackagesInDirectories(testDir)).toMatchSnapshot();
expect(symlinkedDirectories(testDir)).toMatchSnapshot();
});

it("never installs with global style", async () => {
await lernaBootstrap();

expect(NpmUtilities.installInDir).toBeCalledWith(
expect.any(String),
expect.arrayContaining(["foo@^1.0.0"]),
expect.objectContaining({
npmGlobalStyle: false,
}),
expect.any(Function)
);
});
});

describe("with multiple package locations", () => {
Expand Down Expand Up @@ -316,10 +322,16 @@ describe("BootstrapCommand", () => {
await lernaBootstrap();

expect(installedPackagesInDirectories(testDir)).toMatchSnapshot();
expect(NpmUtilities.installInDir.mock.calls[0][2]).toEqual({
npmClient: undefined,
registry: "https://my-secure-registry/npm",
});
expect(NpmUtilities.installInDir).lastCalledWith(
expect.any(String),
expect.arrayContaining(["foo@^1.0.0"]),
expect.objectContaining({
registry: "https://my-secure-registry/npm",
npmClient: undefined,
npmGlobalStyle: false,
}),
expect.any(Function)
);
});
});

Expand Down Expand Up @@ -359,10 +371,14 @@ describe("BootstrapCommand", () => {
await lernaBootstrap();

expect(NpmUtilities.installInDir).not.toBeCalled();
expect(NpmUtilities.installInDirOriginalPackageJson.mock.calls[0][1]).toMatchObject({
"mutex": expect.stringMatching(/^network:\d+$/),
"npmClient": "yarn",
});
expect(NpmUtilities.installInDirOriginalPackageJson).lastCalledWith(
expect.any(String),
expect.objectContaining({
npmClient: "yarn",
mutex: expect.stringMatching(/^network:\d+$/),
}),
expect.any(Function)
);
});
});
});

0 comments on commit a132b6e

Please sign in to comment.