Skip to content

Commit 5b4b2c9

Browse files
committed
fix(publish): Split --skip-* properly, leave working tree clean
BREAKING CHANGE: Previously, gitHead annotations were leftover if `--skip-npm` was passed, despite no actual requirement for that property when no publishing is going on. Now, all publish-related operations are truly skipped with `--skip-npm`, and all git commit/push-related operations are skipped with `--skip-git`. Passing `--skip-npm` will now also always push to remote, which represents a breaking change from 2.x behavior. Thanks @KingScooty for raising the issue!
1 parent 34f3b58 commit 5b4b2c9

File tree

4 files changed

+56
-97
lines changed

4 files changed

+56
-97
lines changed

commands/publish/__tests__/publish-command.test.js

+28-5
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ describe("PublishCommand", () => {
9292
// we've already tested these utilities elsewhere
9393
GitUtilities.isInitialized.mockReturnValue(true);
9494
GitUtilities.getCurrentBranch.mockReturnValue("master");
95+
GitUtilities.getCurrentSHA.mockReturnValue("FULL_SHA");
9596
GitUtilities.getShortSHA.mockReturnValue("deadbeef");
9697
GitUtilities.diffSinceIn.mockReturnValue("");
9798

@@ -117,6 +118,11 @@ describe("PublishCommand", () => {
117118
expect(updatedLernaJson()).toMatchObject({ version: "1.0.1" });
118119
expect(updatedPackageVersions(testDir)).toMatchSnapshot("updated packages");
119120

121+
expect(updatedPackageJSON("package-1")).toEqual({
122+
name: "package-1",
123+
version: "1.0.1",
124+
gitHead: "FULL_SHA", // not versioned
125+
});
120126
expect(updatedPackageJSON("package-2").dependencies).toMatchObject({
121127
"package-1": "^1.0.1",
122128
});
@@ -186,6 +192,11 @@ describe("PublishCommand", () => {
186192

187193
expect(updatedPackageVersions(testDir)).toMatchSnapshot("updated packages");
188194

195+
expect(updatedPackageJSON("package-1")).toEqual({
196+
name: "package-1",
197+
version: "1.0.1",
198+
gitHead: "FULL_SHA", // not versioned
199+
});
189200
expect(updatedPackageJSON("package-2").dependencies).toMatchObject({
190201
"package-1": "^1.0.1",
191202
});
@@ -346,6 +357,11 @@ describe("PublishCommand", () => {
346357
expect(GitUtilities.addTag).not.toBeCalled();
347358
expect(GitUtilities.pushWithTags).not.toBeCalled();
348359

360+
expect(updatedPackageJSON("package-1")).toEqual({
361+
name: "package-1",
362+
version: "1.0.1",
363+
gitHead: "FULL_SHA", // not versioned
364+
});
349365
expect(publishedTagInDirectories(testDir)).toMatchSnapshot("npm published");
350366
});
351367

@@ -368,13 +384,20 @@ describe("PublishCommand", () => {
368384
await lernaPublish(testDir)("--skip-npm");
369385

370386
expect(npmPublish).not.toBeCalled();
371-
expect(npmDistTag.check).not.toBeCalled();
372-
expect(npmDistTag.remove).not.toBeCalled();
373-
expect(npmDistTag.add).not.toBeCalled();
387+
expect(updatedPackageJSON("package-1")).toEqual({
388+
name: "package-1",
389+
version: "1.0.1",
390+
// gitHead not annotated
391+
});
374392

375393
expect(gitCommitMessage()).toEqual("v1.0.1");
376-
// FIXME
377-
// expect(GitUtilities.pushWithTags).lastCalledWith("origin", ["v1.0.1"]);
394+
expect(GitUtilities.pushWithTags).lastCalledWith(
395+
"origin",
396+
["v1.0.1"],
397+
expect.objectContaining({
398+
cwd: testDir,
399+
})
400+
);
378401
});
379402

380403
it("should display a message that npm is skipped", async () => {

commands/publish/index.js

+26-20
Original file line numberDiff line numberDiff line change
@@ -156,16 +156,25 @@ class PublishCommand extends Command {
156156
this.logger.info("execute", "Skipping git commit/push");
157157
}
158158

159-
tasks.push(() => this.resolveLocalDependencyLinks());
160-
tasks.push(() => this.annotateGitHead());
161-
162159
if (this.options.skipNpm) {
163160
this.logger.info("execute", "Skipping publish to registry");
164161
} else {
165162
tasks.push(() => this.publishPackagesToNpm());
166163
}
167164

168-
return pWaterfall(tasks);
165+
if (this.gitEnabled) {
166+
tasks.push(() => this.pushToRemote());
167+
}
168+
169+
return pWaterfall(tasks).then(() => {
170+
this.logger.success("publish", "finished");
171+
});
172+
}
173+
174+
pushToRemote() {
175+
this.logger.info("git", "Pushing tags...");
176+
177+
return GitUtilities.pushWithTags(this.gitRemote, this.tags, this.execOpts);
169178
}
170179

171180
resolveLocalDependencyLinks() {
@@ -203,36 +212,33 @@ class PublishCommand extends Command {
203212
});
204213
}
205214

215+
resetManifestChanges() {
216+
// the package.json files are changed (by gitHead if not --canary)
217+
// and we should always leave the working tree clean
218+
return pReduce(this.project.packageConfigs, (_, pkgGlob) =>
219+
GitUtilities.checkoutChanges(`${pkgGlob}/package.json`, this.execOpts)
220+
);
221+
}
222+
206223
publishPackagesToNpm() {
207224
this.logger.info("publish", "Publishing packages to npm...");
208225

209-
let chain = Promise.resolve().then(() => this.npmPublish());
226+
let chain = Promise.resolve();
210227

211-
// reset since the package.json files are changed (by gitHead if not --canary)
212-
chain = chain.then(() =>
213-
pReduce(this.project.packageConfigs, (_, pkgGlob) =>
214-
GitUtilities.checkoutChanges(`${pkgGlob}/package.json`, this.execOpts)
215-
)
216-
);
228+
chain = chain.then(() => this.resolveLocalDependencyLinks());
229+
chain = chain.then(() => this.annotateGitHead());
230+
chain = chain.then(() => this.npmPublish());
231+
chain = chain.then(() => this.resetManifestChanges());
217232

218233
if (this.options.tempTag) {
219234
chain = chain.then(() => this.npmUpdateAsLatest());
220235
}
221236

222-
if (this.gitEnabled) {
223-
chain = chain.then(() => {
224-
this.logger.info("git", "Pushing tags...");
225-
return GitUtilities.pushWithTags(this.gitRemote, this.tags, this.execOpts);
226-
});
227-
}
228-
229237
return chain.then(() => {
230238
const message = this.packagesToPublish.map(pkg => ` - ${pkg.name}@${pkg.version}`);
231239

232240
output("Successfully published:");
233241
output(message.join(os.EOL));
234-
235-
this.logger.success("publish", "finished");
236242
});
237243
}
238244

integration/__snapshots__/lerna-publish.test.js.snap

+1-67
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ See [Conventional Commits](https://conventionalcommits.org) for commit guideline
260260
]
261261
`;
262262

263-
exports[`lerna publish replaces file: specifier with local version before npm publish but after git commit: committed 1`] = `
263+
exports[`lerna publish replaces file: specifier with local version before npm publish but after git commit 1`] = `
264264
v2.0.0
265265
266266
diff --git a/lerna.json b/lerna.json
@@ -314,59 +314,6 @@ index SHA..SHA 100644
314314
+ "version": "2.0.0",
315315
`;
316316

317-
exports[`lerna publish replaces file: specifier with local version before npm publish but after git commit: unstaged 1`] = `
318-
diff --git a/packages/package-1/package.json b/packages/package-1/package.json
319-
index SHA..SHA 100644
320-
--- a/packages/package-1/package.json
321-
+++ b/packages/package-1/package.json
322-
@@ -7 +7,2 @@
323-
- }
324-
+ },
325-
+ "gitHead": "GIT_HEAD"
326-
diff --git a/packages/package-2/package.json b/packages/package-2/package.json
327-
index SHA..SHA 100644
328-
--- a/packages/package-2/package.json
329-
+++ b/packages/package-2/package.json
330-
@@ -5,2 +5,3 @@
331-
- "package-1": "file:../package-1"
332-
- }
333-
+ "package-1": "^2.0.0"
334-
+ },
335-
+ "gitHead": "GIT_HEAD"
336-
diff --git a/packages/package-3/package.json b/packages/package-3/package.json
337-
index SHA..SHA 100644
338-
--- a/packages/package-3/package.json
339-
+++ b/packages/package-3/package.json
340-
@@ -5,2 +5,3 @@
341-
- "package-2": "file:../package-2"
342-
- }
343-
+ "package-2": "^2.0.0"
344-
+ },
345-
+ "gitHead": "GIT_HEAD"
346-
diff --git a/packages/package-4/package.json b/packages/package-4/package.json
347-
index SHA..SHA 100644
348-
--- a/packages/package-4/package.json
349-
+++ b/packages/package-4/package.json
350-
@@ -5,2 +5,3 @@
351-
- "package-3": "file:../package-3"
352-
- }
353-
+ "package-3": "^2.0.0"
354-
+ },
355-
+ "gitHead": "GIT_HEAD"
356-
diff --git a/packages/package-5/package.json b/packages/package-5/package.json
357-
index SHA..SHA 100644
358-
--- a/packages/package-5/package.json
359-
+++ b/packages/package-5/package.json
360-
@@ -5,3 +5,4 @@
361-
- "package-4": "file:../package-4",
362-
- "package-6": "file:../package-6"
363-
- }
364-
+ "package-4": "^2.0.0",
365-
+ "package-6": "^1.0.0"
366-
+ },
367-
+ "gitHead": "GIT_HEAD"
368-
`;
369-
370317
exports[`lerna publish silences lifecycle scripts with --loglevel=silent 1`] = `
371318
372319
Changes:
@@ -386,7 +333,6 @@ Array [
386333
Object {
387334
change: true,
388335
description: no local dependencies, four local dependents (three transitive),
389-
gitHead: GIT_HEAD,
390336
name: package-1,
391337
version: 2.0.0,
392338
},
@@ -395,7 +341,6 @@ Array [
395341
package-1: ^2.0.0,
396342
},
397343
description: one local dependency, one direct dependent, no transitive dependencies,
398-
gitHead: GIT_HEAD,
399344
name: package-2,
400345
version: 2.0.0,
401346
},
@@ -404,7 +349,6 @@ Array [
404349
package-2: ^2.0.0,
405350
},
406351
description: one local dependency, one direct dependent, one transitive dependency,
407-
gitHead: GIT_HEAD,
408352
name: package-3,
409353
version: 2.0.0,
410354
},
@@ -413,7 +357,6 @@ Array [
413357
package-3: ^2.0.0,
414358
},
415359
description: one local dependency, one direct dependent, two transitive dependencies,
416-
gitHead: GIT_HEAD,
417360
name: package-4,
418361
version: 2.0.0,
419362
},
@@ -422,7 +365,6 @@ Array [
422365
package-4: ^2.0.0,
423366
},
424367
description: one local dependency, no dependents, three transitive dependencies,
425-
gitHead: GIT_HEAD,
426368
name: package-5,
427369
version: 2.0.0,
428370
},
@@ -437,23 +379,20 @@ v1.0.1
437379
exports[`lerna publish updates fixed versions: packages 1`] = `
438380
Array [
439381
Object {
440-
gitHead: GIT_HEAD,
441382
name: package-1,
442383
version: 1.0.1,
443384
},
444385
Object {
445386
dependencies: Object {
446387
package-1: ^1.0.1,
447388
},
448-
gitHead: GIT_HEAD,
449389
name: package-2,
450390
version: 1.0.1,
451391
},
452392
Object {
453393
devDependencies: Object {
454394
package-2: ^1.0.1,
455395
},
456-
gitHead: GIT_HEAD,
457396
name: package-3,
458397
peerDependencies: Object {
459398
package-2: ^1.0.0,
@@ -464,7 +403,6 @@ Array [
464403
dependencies: Object {
465404
package-1: ^0.0.0,
466405
},
467-
gitHead: GIT_HEAD,
468406
name: package-4,
469407
version: 1.0.1,
470408
},
@@ -504,31 +442,27 @@ Publish
504442
exports[`lerna publish updates independent versions: packages 1`] = `
505443
Array [
506444
Object {
507-
gitHead: GIT_HEAD,
508445
name: package-1,
509446
version: 2.0.0,
510447
},
511448
Object {
512449
dependencies: Object {
513450
package-1: ^2.0.0,
514451
},
515-
gitHead: GIT_HEAD,
516452
name: package-2,
517453
version: 3.0.0,
518454
},
519455
Object {
520456
devDependencies: Object {
521457
package-2: ^3.0.0,
522458
},
523-
gitHead: GIT_HEAD,
524459
name: package-3,
525460
version: 4.0.0,
526461
},
527462
Object {
528463
dependencies: Object {
529464
package-1: ^0.0.0,
530465
},
531-
gitHead: GIT_HEAD,
532466
name: package-4,
533467
version: 5.0.0,
534468
},

integration/lerna-publish.test.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,7 @@ describe("lerna publish", () => {
148148

149149
expect(
150150
await execa.stdout("git", ["show", "--unified=0", "--ignore-space-at-eol", "--format=%s"], { cwd })
151-
).toMatchSnapshot("committed");
152-
153-
expect(
154-
await execa.stdout("git", ["diff", "--unified=0", "--ignore-space-at-eol"], { cwd })
155-
).toMatchSnapshot("unstaged");
151+
).toMatchSnapshot();
156152
});
157153

158154
test("calls lifecycle scripts", async () => {

0 commit comments

Comments
 (0)