feat(npm-publish): Use libnpm/publish instead of subprocess execution

Author: evocateur

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

@@ -314,6 +314,17 @@ Set {
   });
 
   describe("--registry", () => {
+    const confWithRegistry = registry =>
+      expect.objectContaining({
+        sources: expect.objectContaining({
+          cli: {
+            data: expect.objectContaining({
+              registry,
+            }),
+          },
+        }),
+      });
+
     it("passes registry to npm commands", async () => {
       const testDir = await initFixture("normal");
       const registry = "https://my-private-registry";
@@ -322,8 +333,8 @@ Set {
 
       expect(npmPublish).toHaveBeenCalledWith(
         expect.objectContaining({ name: "package-1" }),
-        undefined, // dist-tag
-        expect.objectContaining({ registry })
+        "latest", // dist-tag
+        confWithRegistry(registry)
       );
     });
 
@@ -335,8 +346,8 @@ Set {
 
       expect(npmPublish).toHaveBeenCalledWith(
         expect.objectContaining({ name: "package-1" }),
-        undefined, // dist-tag
-        expect.objectContaining({ registry: "https://registry.npmjs.org/" })
+        "latest", // dist-tag
+        confWithRegistry("https://registry.npmjs.org/")
       );
 
       const logMessages = loggingOutput("warn");

commands/publish/__tests__/publish-lifecycle-scripts.test.js

@@ -31,12 +31,6 @@ describe("lifecycle scripts", () => {
       expect(runLifecycle).toHaveBeenCalledWith(expect.objectContaining({ name: "lifecycle" }), script);
     });
 
-    // all leaf package lifecycles _EXCEPT_ postpublish are called in packDirectory()
-    expect(runLifecycle).toHaveBeenCalledWith(
-      expect.objectContaining({ name: "package-1" }),
-      expect.stringMatching("postpublish")
-    );
-
     // package-2 lacks version lifecycle scripts
     expect(runLifecycle).not.toHaveBeenCalledWith(
       expect.objectContaining({ name: "package-2" }),
@@ -56,7 +50,6 @@ describe("lifecycle scripts", () => {
       ["lifecycle", "prepublishOnly"],
       ["lifecycle", "prepack"],
       ["lifecycle", "postpack"],
-      ["package-1", "postpublish"],
       ["lifecycle", "postpublish"],
     ]);
 

package-lock.json

@@ -1,13 +1,13 @@
 "use strict";
 
-jest.mock("@lerna/child-process");
-jest.mock("@lerna/has-npm-version");
-jest.mock("@lerna/log-packed");
+jest.mock("@lerna/run-lifecycle");
+jest.mock("libnpm/publish");
 jest.mock("fs-extra");
 
 // mocked modules
-const ChildProcessUtilities = require("@lerna/child-process");
 const fs = require("fs-extra");
+const publish = require("libnpm/publish");
+const runLifecycle = require("@lerna/run-lifecycle");
 
 // helpers
 const path = require("path");
@@ -17,8 +17,12 @@ const Package = require("@lerna/package");
 const npmPublish = require("..");
 
 describe("npm-publish", () => {
+  const mockTarData = Buffer.from("MOCK");
+
+  fs.readFile.mockImplementation(() => Promise.resolve(mockTarData));
   fs.remove.mockResolvedValue();
-  ChildProcessUtilities.exec.mockResolvedValue();
+  publish.mockResolvedValue();
+  runLifecycle.mockResolvedValue();
 
   const rootPath = path.normalize("/test");
   const pkg = new Package(
@@ -27,96 +31,84 @@ describe("npm-publish", () => {
     rootPath
   );
 
-  // technically decorated in npmPack, stubbed here
+  // technically decorated in ../../commands/publish, stubbed here
   pkg.tarball = {
     filename: "test-1.10.100.tgz",
   };
 
-  it("runs npm publish in a directory with --tag support", async () => {
-    const result = await npmPublish(pkg, "published-tag", { npmClient: "npm" });
+  it("pipelines input package", async () => {
+    const opts = new Map();
+    const result = await npmPublish(pkg, "latest", opts);
 
     expect(result).toBe(pkg);
-    expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
-      "npm",
-      ["publish", "--ignore-scripts", "--tag", "published-tag", "test-1.10.100.tgz"],
-      {
-        cwd: pkg.location,
-        env: {},
-        pkg,
-      }
-    );
-    expect(fs.remove).toHaveBeenLastCalledWith(path.join(pkg.location, pkg.tarball.filename));
   });
 
-  it("does not pass --tag when none present (npm default)", async () => {
-    await npmPublish(pkg, undefined, { npmClient: "npm" });
-
-    expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
-      "npm",
-      ["publish", "--ignore-scripts", "test-1.10.100.tgz"],
-      {
-        cwd: pkg.location,
-        env: {},
-        pkg,
-      }
-    );
+  it("calls libnpm/publish with correct arguments", async () => {
+    const opts = new Map();
+
+    await npmPublish(pkg, "published-tag", opts);
+
+    expect(publish).toHaveBeenCalledWith({ name: "test", version: "1.10.100" }, mockTarData, {
+      projectScope: "test",
+      tag: "published-tag",
+    });
   });
 
-  it("trims trailing whitespace in tag parameter", async () => {
-    await npmPublish(pkg, "trailing-tag ", { npmClient: "npm" });
-
-    expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
-      "npm",
-      ["publish", "--ignore-scripts", "--tag", "trailing-tag", "test-1.10.100.tgz"],
-      {
-        cwd: pkg.location,
-        env: {},
-        pkg,
-      }
+  it("falls back to opts.tag for dist-tag", async () => {
+    const opts = new Map([["tag", "custom-default"]]);
+
+    await npmPublish(pkg, undefined, opts);
+
+    expect(publish).toHaveBeenCalledWith(
+      { name: "test", version: "1.10.100" },
+      mockTarData,
+      expect.objectContaining({ tag: "custom-default" })
     );
   });
 
-  it("supports custom registry", async () => {
-    const registry = "https://custom-registry/npmPublish";
-
-    await npmPublish(pkg, "custom-registry", { npmClient: "npm", registry });
-
-    expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
-      "npm",
-      ["publish", "--ignore-scripts", "--tag", "custom-registry", "test-1.10.100.tgz"],
-      {
-        cwd: pkg.location,
-        env: {
-          npm_config_registry: registry,
-        },
-        pkg,
-      }
+  it("falls back to default tag", async () => {
+    await npmPublish(pkg);
+
+    expect(publish).toHaveBeenCalledWith(
+      { name: "test", version: "1.10.100" },
+      mockTarData,
+      expect.objectContaining({ tag: "latest" })
     );
   });
 
-  describe("with npmClient yarn", () => {
-    it("appends --new-version to avoid interactive prompt", async () => {
-      await npmPublish(pkg, "yarn-publish", { npmClient: "yarn" });
-
-      expect(ChildProcessUtilities.exec).toHaveBeenLastCalledWith(
-        "yarn",
-        [
-          "publish",
-          "--ignore-scripts",
-          "--tag",
-          "yarn-publish",
-          "--new-version",
-          pkg.version,
-          "--non-interactive",
-          "--no-git-tag-version",
-          "test-1.10.100.tgz",
-        ],
-        {
-          cwd: pkg.location,
-          env: {},
-          pkg,
-        }
-      );
+  it("calls publish lifecycles", async () => {
+    await npmPublish(pkg, "lifecycles");
+
+    // figgy-pudding Proxy hanky-panky defeats jest wizardry
+    const aFiggyPudding = expect.objectContaining({ __isFiggyPudding: true });
+
+    expect(runLifecycle).toHaveBeenCalledWith(pkg, "publish", aFiggyPudding);
+    expect(runLifecycle).toHaveBeenCalledWith(pkg, "postpublish", aFiggyPudding);
+
+    runLifecycle.mock.calls.forEach(args => {
+      const pud = args.slice().pop();
+
+      expect(pud.toJSON()).toMatchObject({
+        projectScope: "test",
+        tag: "lifecycles",
+      });
     });
   });
+
+  it("omits opts.logstream", async () => {
+    const opts = new Map([["logstream", "SKIPPED"]]);
+
+    await npmPublish(pkg, "canary", opts);
+
+    const pud = runLifecycle.mock.calls.pop().pop();
+
+    expect(pud.toJSON()).not.toHaveProperty("logstream");
+  });
+
+  it("removes tarball after success", async () => {
+    const opts = new Map();
+    await npmPublish(pkg, "latest", opts);
+
+    expect(fs.remove).toHaveBeenLastCalledWith(path.join(pkg.location, pkg.tarball.filename));
+  });
 });

utils/npm-publish/__tests__/npm-publish.test.js

@@ -1,38 +1,53 @@
 "use strict";
 
 const fs = require("fs-extra");
-const log = require("libnpm/log");
 const path = require("path");
-
-const ChildProcessUtilities = require("@lerna/child-process");
-const getExecOpts = require("@lerna/get-npm-exec-opts");
+const log = require("libnpm/log");
+const publish = require("libnpm/publish");
+const figgyPudding = require("figgy-pudding");
+const runLifecycle = require("@lerna/run-lifecycle");
 
 module.exports = npmPublish;
 
-function npmPublish(pkg, tag, { npmClient, registry }) {
+const PublishConfig = figgyPudding(
+  {
+    "dry-run": { default: false },
+    dryRun: "dry-run",
+    tag: { default: "latest" },
+  },
+  {
+    other(key) {
+      // allow any other keys _except_ circular objects
+      return key !== "log" && key !== "logstream";
+    },
+  }
+);
+
+function npmPublish(pkg, tag, _opts) {
   log.verbose("publish", pkg.name);
 
-  const distTag = tag && tag.trim();
-  const opts = getExecOpts(pkg, registry);
-  const args = ["publish", "--ignore-scripts"];
+  const deets = { projectScope: pkg.name };
 
-  if (distTag) {
-    args.push("--tag", distTag);
+  if (tag) {
+    deets.tag = tag;
   }
 
-  if (npmClient === "yarn") {
-    // skip prompt for new version, use existing instead
-    // https://yarnpkg.com/en/docs/cli/publish#toc-yarn-publish-new-version
-    args.push("--new-version", pkg.version, "--non-interactive", "--no-git-tag-version");
-    // yarn also needs to be told to stop creating git tags: https://git.io/fAr1P
+  const opts = PublishConfig(_opts, deets);
+  const tarFilePath = path.join(pkg.location, pkg.tarball.filename);
+
+  let chain = Promise.resolve();
+
+  if (!opts.dryRun) {
+    chain = chain.then(() => fs.readFile(tarFilePath));
+    chain = chain.then(tarData => publish(pkg.toJSON(), tarData, opts.toJSON()));
   }
 
-  // always add tarball file, created by npmPack()
-  args.push(pkg.tarball.filename);
+  chain = chain.then(() => runLifecycle(pkg, "publish", opts));
+  chain = chain.then(() => runLifecycle(pkg, "postpublish", opts));
+
+  // don't leave the generated tarball hanging around after success
+  chain = chain.then(() => fs.remove(tarFilePath));
 
-  log.silly("exec", npmClient, args);
-  return ChildProcessUtilities.exec(npmClient, args, opts).then(() =>
-    // don't leave the generated tarball hanging around after success
-    fs.remove(path.join(pkg.location, pkg.tarball.filename)).then(() => pkg)
-  );
+  // pipelined Package instance
+  return chain.then(() => pkg);
 }

utils/npm-publish/npm-publish.js

@@ -30,8 +30,8 @@
     "test": "echo \"Run tests from root\" && exit 1"
   },
   "dependencies": {
-    "@lerna/child-process": "file:../../core/child-process",
-    "@lerna/get-npm-exec-opts": "file:../get-npm-exec-opts",
+    "@lerna/run-lifecycle": "file:../run-lifecycle",
+    "figgy-pudding": "^3.5.1",
     "fs-extra": "^7.0.0",
     "libnpm": "^2.0.1"
   }