Skip to content

Commit

Permalink
fix(publish): Improve npm pack experience
Browse files Browse the repository at this point in the history
- It works now
- Including integration tests (very slow though :P)
- Logging is good
- Package instances now expose `.rootPath` read-only property
- Removed pkg.tarball, now decorated by npmPack() from JSON result
  • Loading branch information
evocateur committed Aug 9, 2018
1 parent 9c767ac commit 627cfc2
Show file tree
Hide file tree
Showing 13 changed files with 212 additions and 79 deletions.
11 changes: 8 additions & 3 deletions commands/__mocks__/@lerna/npm-publish.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ const mockNpmPublish = jest.fn((pkg, tag) => {
return Promise.resolve();
});

const mockNpmPack = jest.fn(pkg => {
packed.add(pkg.name);
const mockNpmPack = jest.fn((rootManifest, packages) => {
packages.forEach(pkg => {
packed.add(pkg.name);

return Promise.resolve(pkg.tarball);
// simulate decoration after npm pack
pkg.tarball = `${pkg.name}-MOCKED.tgz`;
});

return Promise.resolve();
});

// a convenient format for assertions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"postversion": "echo postversion-package-1",
"prepublish": "echo prepublish-package-1",
"prepare": "echo prepare-package-1",
"prepublishOnly": "echo prepublish-package-1",
"prepublishOnly": "echo prepublishOnly-package-1",
"postpublish": "echo postpublish-package-1"
}
}
8 changes: 8 additions & 0 deletions commands/publish/__tests__/publish-command.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,14 @@ describe("PublishCommand", () => {

await lernaPublish(testDir)();

expect(npmPublish.packed).toMatchInlineSnapshot(`
Set {
"package-1",
"package-3",
"package-4",
"package-2",
}
`);
expect(npmPublish.order()).toEqual([
"package-1",
"package-3",
Expand Down
9 changes: 7 additions & 2 deletions commands/publish/__tests__/publish-lifecycle-scripts.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ describe("lifecycle scripts", () => {
expect(runLifecycle).toHaveBeenCalledWith(expect.objectContaining({ name: "lifecycle" }), script);
});

// all leaf package lifecycles are called by npm pack
// all leaf package lifecycles _EXCEPT_ postpublish are called by npm pack
expect(runLifecycle).not.toHaveBeenCalledWith(
expect.objectContaining({ name: "package-1" }),
expect.stringMatching(/(prepare|prepublishOnly|postpublish)/)
expect.stringMatching(/(prepare|prepublishOnly)/)
);
expect(runLifecycle).toHaveBeenCalledWith(
expect.objectContaining({ name: "package-1" }),
expect.stringMatching("postpublish")
);

// package-2 lacks version lifecycle scripts
Expand All @@ -52,6 +56,7 @@ describe("lifecycle scripts", () => {
// publish-specific
["lifecycle", "prepare"],
["lifecycle", "prepublishOnly"],
["package-1", "postpublish"],
["lifecycle", "postpublish"],
]);
});
Expand Down
30 changes: 18 additions & 12 deletions commands/publish/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"use strict";

const os = require("os");
const fs = require("fs-extra");
const path = require("path");
const pFinally = require("p-finally");
const pMap = require("p-map");
Expand Down Expand Up @@ -373,9 +374,9 @@ class PublishCommand extends Command {
}

chain = chain.then(() =>
runParallelBatches(this.batchedPackages, this.concurrency, pkg =>
npmPublish.npmPack(pkg).then(() => {
tracker.completeWork(1);
pReduce(this.batchedPackages, (_, batch) =>
npmPublish.npmPack(this.project.manifest, batch).then(() => {
tracker.completeWork(batch.length);
})
)
);
Expand All @@ -399,15 +400,20 @@ class PublishCommand extends Command {

chain = chain.then(() =>
runParallelBatches(this.batchedPackages, this.concurrency, pkg =>
npmPublish(pkg, distTag, this.npmConfig).then(() => {
tracker.info("published", pkg.name);

if (this.options.requireScripts) {
this.execScript(pkg, "postpublish");
}

tracker.completeWork(1);
})
npmPublish(pkg, distTag, this.npmConfig)
// postpublish is _not_ run when publishing a tarball
.then(() => this.runPackageLifecycle(pkg, "postpublish"))
// don't leave the generated tarball hanging around
.then(() => fs.remove(path.join(pkg.rootPath, pkg.tarball)))
.then(() => {
tracker.info("published", pkg.name);

if (this.options.requireScripts) {
this.execScript(pkg, "postpublish");
}

tracker.completeWork(1);
})
)
);

Expand Down
19 changes: 7 additions & 12 deletions core/package/__tests__/core-package.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ describe("Package", () => {
});
});

describe("get .rootPath", () => {
it("should return the rootPath", () => {
const pkg = factory({ name: "get-rootPath" });
expect(pkg.rootPath).toBe(path.normalize("/root"));
});
});

describe("get .version", () => {
it("should return the version", () => {
const pkg = factory({ version: "1.0.0" });
Expand Down Expand Up @@ -144,18 +151,6 @@ describe("Package", () => {
});
});

describe("get .tarball", () => {
it("generates the destination filename of npm pack output", () => {
const pkg = factory({ name: "pack-me", version: "1.2.3" });
expect(pkg.tarball).toBe("pack-me-1.2.3.tgz");
});

it("normalizes package scopes", () => {
const pkg = factory({ name: "@scoped/pack-me", version: "4.5.6" });
expect(pkg.tarball).toBe("scoped-pack-me-4.5.6.tgz");
});
});

describe(".toJSON()", () => {
it("should return clone of internal package for serialization", () => {
const json = {
Expand Down
20 changes: 3 additions & 17 deletions core/package/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,6 @@ function binSafeName({ name, scope }) {
return scope ? name.substring(scope.length + 1) : name;
}

function packedFileName(pkg) {
// copied almost verbatim from https://git.io/fNobb
const name =
pkg.name[0] === "@"
? // scoped packages get special treatment
pkg.name.substr(1).replace(/\//g, "-")
: pkg.name;

return `${name}-${pkg.version}.tgz`;
}

// package.json files are not that complicated, so this is intentionally naïve
function shallowCopy(json) {
return Object.keys(json).reduce((obj, key) => {
Expand Down Expand Up @@ -57,6 +46,9 @@ class Package {
resolved: {
value: resolved,
},
rootPath: {
value: rootPath,
},
// mutable
version: {
get() {
Expand Down Expand Up @@ -127,12 +119,6 @@ class Package {
serialize: {
value: () => writePkg(this.manifestLocation, pkg),
},
// compute result of `npm pack` filename
tarball: {
get() {
return packedFileName(pkg);
},
},
});
}

Expand Down
2 changes: 2 additions & 0 deletions helpers/cli-runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ function runner(cwd, env) {
const opts = {
cwd,
env: Object.assign({ CI: "true" }, env),
// when debugging integration test snapshots, uncomment next line
// stdio: ["ignore", "inherit", "inherit"],
};

return (...args) => execa("node", [LERNA_BIN].concat(args), opts);
Expand Down
12 changes: 6 additions & 6 deletions integration/__snapshots__/lerna-publish.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ prepare-root
prepublishOnly-root
> package-1@1.1.0 prepublish __TEST_ROOTDIR__/packages/package-1
> echo prepublish-package-1
prepublish-package-1
> package-1@1.1.0 prepare __TEST_ROOTDIR__/packages/package-1
> echo prepare-package-1
prepare-package-1
> package-1@1.1.0 prepublishOnly __TEST_ROOTDIR__/packages/package-1
> echo prepublish-package-1
prepublish-package-1
> package-1@1.1.0 postpublish __TEST_ROOTDIR__/packages/package-1
> echo postpublish-package-1
Expand Down Expand Up @@ -380,8 +380,8 @@ postversion-package-1
postversion-root
prepare-root
prepublishOnly-root
prepare-package-1
prepublish-package-1
prepare-package-1
postpublish-package-1
postpublish-root
Successfully published:
Expand Down
1 change: 1 addition & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 627cfc2

Please sign in to comment.