diff --git a/commands/publish/__tests__/publish-canary.test.js b/commands/publish/__tests__/publish-canary.test.js index fbb36cc1bf..e99ee0499f 100644 --- a/commands/publish/__tests__/publish-canary.test.js +++ b/commands/publish/__tests__/publish-canary.test.js @@ -79,9 +79,9 @@ test("publish --canary", async () => { expect(npmPublish.registry).toMatchInlineSnapshot(` Map { "package-1" => "canary", - "package-3" => "canary", "package-4" => "canary", "package-2" => "canary", + "package-3" => "canary", } `); expect(writePkg.updatedVersions()).toMatchInlineSnapshot(` diff --git a/commands/publish/__tests__/publish-command.test.js b/commands/publish/__tests__/publish-command.test.js index 5d4434870c..5a4f73d5f1 100644 --- a/commands/publish/__tests__/publish-command.test.js +++ b/commands/publish/__tests__/publish-command.test.js @@ -101,24 +101,24 @@ describe("PublishCommand", () => { expect(packDirectory.registry).toMatchInlineSnapshot(` Set { "package-1", - "package-3", "package-4", "package-2", + "package-3", } `); expect(npmPublish.registry).toMatchInlineSnapshot(` Map { "package-1" => "latest", - "package-3" => "latest", "package-4" => "latest", "package-2" => "latest", + "package-3" => "latest", } `); expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", // package-5 is private ]); expect(npmDistTag.remove).not.toHaveBeenCalled(); @@ -142,9 +142,9 @@ Map { expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", // package-5 is private ]); }); @@ -169,6 +169,21 @@ Map { }); describe("--graph-type", () => { + it("produces a topological ordering that _includes_ devDependencies when value is not set", async () => { + const cwd = await initFixture("normal"); + + await lernaPublish(cwd)(); + + expect(npmPublish.order()).toEqual([ + "package-1", + "package-4", + "package-2", + // package-3 has a peer/devDependency on package-2 + "package-3", + // package-5 is private + ]); + }); + it("produces a topological ordering that _includes_ devDependencies when value is 'all'", async () => { const cwd = await initFixture("normal"); @@ -184,6 +199,28 @@ Map { ]); }); + it("produces a topological ordering that _excludes_ devDependencies when value is 'dependencies' (DEPRECATED)", async () => { + const cwd = await initFixture("normal"); + + await lernaPublish(cwd)("--graph-type", "dependencies"); + + expect(npmPublish.order()).toEqual([ + "package-1", + // package-3 has a peer/devDependency on package-2 + "package-3", + "package-4", + "package-2", + // package-5 is private + ]); + + const logMessages = loggingOutput("warn"); + expect(logMessages).toMatchInlineSnapshot(` + Array [ + "--graph-type=dependencies is deprecated and will be removed in lerna v6. If you have a use-case you feel requires it please open an issue to discuss: https://github.com/lerna/lerna/issues/new/choose", + ] + `); + }); + it("throws an error when value is _not_ 'all' or 'dependencies'", async () => { const testDir = await initFixture("normal"); const command = lernaPublish(testDir)("--graph-type", "poopy-pants"); @@ -192,6 +229,22 @@ Map { }); }); + describe("--no-sort", () => { + it("produces a lexical ordering when --no-sort is set", async () => { + const cwd = await initFixture("normal"); + + await lernaPublish(cwd)("--no-sort"); + + expect(npmPublish.order()).toEqual([ + "package-1", + "package-2", + "package-3", + "package-4", + // package-5 is private + ]); + }); + }); + describe("--otp", () => { getOneTimePassword.mockImplementation(() => Promise.resolve("654321")); @@ -317,24 +370,24 @@ Map { expect(packDirectory.registry).toMatchInlineSnapshot(` Set { "package-1", - "package-3", "package-4", "package-2", + "package-3", } `); expect(npmPublish.registry).toMatchInlineSnapshot(` Map { "package-1" => "latest", - "package-3" => "latest", "package-4" => "latest", "package-2" => "latest", + "package-3" => "latest", } `); expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", // package-5 is private ]); expect(npmDistTag.remove).not.toHaveBeenCalled(); diff --git a/commands/publish/__tests__/publish-from-git.test.js b/commands/publish/__tests__/publish-from-git.test.js index add3aaf0b2..5c068eb10b 100644 --- a/commands/publish/__tests__/publish-from-git.test.js +++ b/commands/publish/__tests__/publish-from-git.test.js @@ -43,9 +43,29 @@ describe("publish from-git", () => { expect(output.logged()).toMatch("Found 4 packages to publish:"); expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", + // package-5 is private + ]); + }); + + it("publishes tagged packages, lexically sorted when --no-sort is present", async () => { + const cwd = await initFixture("normal"); + + await gitTag(cwd, "v1.0.0"); + await lernaPublish(cwd)("from-git", "--no-sort"); + + // called from chained describeRef() + expect(throwIfUncommitted).toHaveBeenCalled(); + + expect(promptConfirmation).toHaveBeenLastCalledWith("Are you sure you want to publish these packages?"); + expect(output.logged()).toMatch("Found 4 packages to publish:"); + expect(npmPublish.order()).toEqual([ + "package-1", + "package-2", + "package-3", + "package-4", // package-5 is private ]); }); @@ -64,9 +84,9 @@ describe("publish from-git", () => { expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", // package-5 is private ]); }); @@ -79,9 +99,9 @@ describe("publish from-git", () => { expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", // package-5 is private ]); }); diff --git a/commands/publish/__tests__/publish-from-package.test.js b/commands/publish/__tests__/publish-from-package.test.js index 4021df07d1..5d12b6f6a0 100644 --- a/commands/publish/__tests__/publish-from-package.test.js +++ b/commands/publish/__tests__/publish-from-package.test.js @@ -55,9 +55,25 @@ describe("publish from-package", () => { expect(npmPublish.order()).toEqual([ "package-1", - "package-3", "package-4", "package-2", + "package-3", + // package-5 is private + ]); + }); + + it("publishes unpublished independent packages, lexically sorted when --no-sort is present", async () => { + const cwd = await initFixture("independent"); + + getUnpublishedPackages.mockImplementationOnce((packageGraph) => Array.from(packageGraph.values())); + + await lernaPublish(cwd)("from-package", "--no-sort"); + + expect(npmPublish.order()).toEqual([ + "package-1", + "package-2", + "package-3", + "package-4", // package-5 is private ]); }); diff --git a/commands/publish/index.js b/commands/publish/index.js index f123f352ef..d4643b9b4f 100644 --- a/commands/publish/index.js +++ b/commands/publish/index.js @@ -58,6 +58,9 @@ class PublishCommand extends Command { configureProperties() { super.configureProperties(); + // For publish we want to enable topological sorting by default, but allow users to override with --no-sort + this.toposort = this.options.sort !== false; + // Defaults are necessary here because yargs defaults // override durable options provided by a config file const { @@ -102,6 +105,13 @@ class PublishCommand extends Command { ); } + if (this.options.graphType === "dependencies") { + this.logger.warn( + "graph-type", + "--graph-type=dependencies is deprecated and will be removed in lerna v6. If you have a use-case you feel requires it please open an issue to discuss: https://github.com/lerna/lerna/issues/new/choose" + ); + } + if (this.options.skipNpm) { // TODO: remove in next major release this.logger.warn("deprecated", "Instead of --skip-npm, call `lerna version` directly"); @@ -626,15 +636,21 @@ class PublishCommand extends Command { } topoMapPackages(mapper) { - // we don't respect --no-sort here, sorry return runTopologically(this.packagesToPublish, mapper, { concurrency: this.concurrency, rejectCycles: this.options.rejectCycles, - // By default, do not include devDependencies in the graph because it would - // increase the chance of dependency cycles, causing less-than-ideal order. - // If the user has opted-in to --graph-type=all (or "graphType": "all" in lerna.json), - // devDependencies _will_ be included in the graph construction. - graphType: this.options.graphType === "all" ? "allDependencies" : "dependencies", + /** + * Previously `publish` had unique default behavior for graph creation vs other commands: it would only consider dependencies when finding + * edges by default (i.e. relationships between packages specified via devDependencies would be ignored). It was documented to be the case + * in order to try and reduce the chance of dependency cycles. + * + * We are removing this behavior altogether in v6 because we do not want to have different ways of constructing the graph, + * only different ways of utilizing it (e.g. --no-sort vs topological sort). + * + * Therefore until we remove graphType altogether in v6, we provide a way for users to opt into the old default behavior + * by setting the `graphType` option to `dependencies`. + */ + graphType: this.options.graphType === "dependencies" ? "dependencies" : "allDependencies", }); } @@ -676,7 +692,12 @@ class PublishCommand extends Command { ].filter(Boolean) ); - chain = chain.then(() => this.topoMapPackages(mapper)); + chain = chain.then(() => { + if (this.toposort) { + return this.topoMapPackages(mapper); + } + return pMap(this.packagesToPublish, mapper, { concurrency: this.concurrency }); + }); chain = chain.then(() => removeTempLicenses(this.packagesToBeLicensed)); @@ -729,7 +750,12 @@ class PublishCommand extends Command { ].filter(Boolean) ); - chain = chain.then(() => this.topoMapPackages(mapper)); + chain = chain.then(() => { + if (this.toposort) { + return this.topoMapPackages(mapper); + } + return pMap(this.packagesToPublish, mapper, { concurrency: this.concurrency }); + }); if (!this.hasRootedLeaf) { // cyclical "publish" lifecycles are automatically skipped @@ -772,7 +798,12 @@ class PublishCommand extends Command { }); }; - chain = chain.then(() => this.topoMapPackages(mapper)); + chain = chain.then(() => { + if (this.toposort) { + return this.topoMapPackages(mapper); + } + return pMap(this.packagesToPublish, mapper, { concurrency: this.concurrency }); + }); return chain.finally(() => tracker.finish()); } diff --git a/core/lerna/schemas/lerna-schema.json b/core/lerna/schemas/lerna-schema.json index d5f6806b69..cfba84c192 100644 --- a/core/lerna/schemas/lerna-schema.json +++ b/core/lerna/schemas/lerna-schema.json @@ -1600,8 +1600,10 @@ }, "graphType": { "type": "string", - "enum": ["all", "depencencies"], - "description": "For `lerna publish`, the type of dependency to use when determining package hierarchy." + "enum": ["all", "dependencies"], + "description": "DEPRECATED: For `lerna publish`, if you want to ignore devDependencies when considering the package graph you can set this property equal to 'dependencies'. This will be removed in v6 of lerna.", + "default": "all", + "deprecated": true }, "otp": { "type": "string",