Rework publishing to work with all popular package managers

packages/cli/src/commands/publish/npm-utils.ts

@@ -9,6 +9,7 @@ import semver from "semver";
 import { askQuestion } from "../../utils/cli-utilities";
 import isCI from "../../utils/isCI";
 import { TwoFactorState } from "../../utils/types";
+import { getLastJsonObjectFromString } from "../../utils/getLastJsonObjectFromString";
 
 const npmRequestLimit = pLimit(40);
 const npmPublishLimit = pLimit(10);
@@ -35,23 +36,33 @@ function getCorrectRegistry(packageJson?: PackageJSON): string {
 
 async function getPublishTool(
   cwd: string
-): Promise<{ name: "npm" } | { name: "pnpm"; shouldAddNoGitChecks: boolean }> {
+): Promise<{ name: "npm" | "pnpm" | "yarn"; args: string[]; flags: string[] }> {
   const pm = await preferredPM(cwd);
-  if (!pm || pm.name !== "pnpm") return { name: "npm" };
-  try {
-    let result = await spawn("pnpm", ["--version"], { cwd });
-    let version = result.stdout.toString().trim();
-    let parsed = semver.parse(version);
-    return {
-      name: "pnpm",
-      shouldAddNoGitChecks:
-        parsed?.major === undefined ? false : parsed.major >= 5
-    };
-  } catch (e) {
-    return {
-      name: "pnpm",
-      shouldAddNoGitChecks: false
-    };
+  if (!pm) {
+    return { name: "npm", args: ["publish"], flags: [] };
+  }
+  const version = (await spawn(pm.name, ["--version"], { cwd })).stdout
+    .toString()
+    .trim();
+
+  const parsed = semver.parse(version)!;
+
+  switch (pm.name) {
+    case "npm":
+      return { name: "npm", args: ["publish"], flags: [] };
+    case "pnpm":
+      if (parsed.major < 5) {
+        return { name: "pnpm", args: ["publish"], flags: [] };
+      }
+      return { name: "pnpm", args: ["publish"], flags: ["--no-git-checks"] };
+    case "yarn":
+      // classic yarn doesn't do anything special when publishing, let's stick to the npm client in such a case
+      if (parsed.major < 2) {
+        return { name: "npm", args: ["publish"], flags: [] };
+      }
+      return { name: "yarn", args: ["npm", "publish"], flags: [] };
+    default:
+      return { name: "npm", args: ["publish"], flags: [] };
   }
 }
 
@@ -143,6 +154,14 @@ export let getOtpCode = async (twoFactorState: TwoFactorState) => {
   return askForOtpCode(twoFactorState);
 };
 
+const isOtpError = (error: any) => {
+  // The first case is no 2fa provided, the second is when the 2fa is wrong (timeout or wrong words)
+  return (
+    error.code === "EOTP" ||
+    (error.code === "E401" && error.detail.includes("--otp=<code>"))
+  );
+};
+
 // we have this so that we can do try a publish again after a publish without
 // the call being wrapped in the npm request limit and causing the publishes to potentially never run
 async function internalPublish(
@@ -151,57 +170,81 @@ async function internalPublish(
   twoFactorState: TwoFactorState
 ): Promise<{ published: boolean }> {
   let publishTool = await getPublishTool(opts.cwd);
+  let shouldHandleOtp =
+    !isCI &&
+    // yarn berry doesn't accept `--otp` and it asks for it on its own
+    publishTool.name !== "yarn";
   let publishFlags = opts.access ? ["--access", opts.access] : [];
   publishFlags.push("--tag", opts.tag);
-  if ((await twoFactorState.isRequired) && !isCI) {
+
+  if (shouldHandleOtp && (await twoFactorState.isRequired)) {
     let otpCode = await getOtpCode(twoFactorState);
     publishFlags.push("--otp", otpCode);
   }
-  if (publishTool.name === "pnpm" && publishTool.shouldAddNoGitChecks) {
-    publishFlags.push("--no-git-checks");
-  }
 
   // Due to a super annoying issue in yarn, we have to manually override this env variable
   // See: https://github.com/yarnpkg/yarn/issues/2935#issuecomment-355292633
   const envOverride = {
     npm_config_registry: getCorrectRegistry()
   };
-  let { stdout } = await spawn(
+  let { code, stdout, stderr } = await spawn(
     publishTool.name,
-    ["publish", opts.cwd, "--json", ...publishFlags],
+    [
+      ...publishTool.args,
+      opts.cwd,
+      "--json",
+      ...publishFlags,
+      ...publishTool.flags
+    ],
     {
       env: Object.assign({}, process.env, envOverride)
     }
   );
-  // New error handling. NPM's --json option is included alongside the `prepublish and
-  // `postpublish` contents in terminal. We want to handle this as best we can but it has
-  // some struggles
-  // Note that both pre and post publish hooks are printed before the json out, so this works.
-  let json = jsonParse(stdout.toString().replace(/[^{]*/, ""));
-
-  if (json.error) {
-    // The first case is no 2fa provided, the second is when the 2fa is wrong (timeout or wrong words)
-    if (
-      (json.error.code === "EOTP" ||
-        (json.error.code === "E401" &&
-          json.error.detail.includes("--otp=<code>"))) &&
-      !isCI
-    ) {
-      if (twoFactorState.token !== null) {
-        // the current otp code must be invalid since it errored
-        twoFactorState.token = null;
+
+  if (code !== 0) {
+    // yarn berry doesn't support --json and we don't attempt to parse its output to a machine-readable format
+    if (publishTool.name === "yarn") {
+      const output = stdout
+        .toString()
+        .trim()
+        .split("\n")
+        // this filters out "unnamed" logs: https://yarnpkg.com/advanced/error-codes/#yn0000---unnamed
+        // this includes a list of packed files and the "summary output" like: "Failed with errors in 0s 75ms"
+        // those are not that interesting so we reduce the noise by dropping them
+        .filter(line => !/YN0000:/.test(line))
+        .join("\n");
+      error(`an error occurred while publishing ${pkgName}:`, `\n${output}`);
+      return { published: false };
+    }
+
+    // NPM's --json output is included alongside the `prepublish` and `postpublish` output in terminal
+    // We want to handle this as best we can but it has some struggles:
+    // - output of those lifecycle scripts can contain JSON
+    // - npm7 has switched to printing `--json` errors to stderr (https://github.com/npm/cli/commit/1dbf0f9bb26ba70f4c6d0a807701d7652c31d7d4)
+    // Note that the `--json` output is always printed at the end so this should work
+    let json =
+      getLastJsonObjectFromString(stderr.toString()) ||
+      getLastJsonObjectFromString(stdout.toString());
+
+    if (json.error) {
+      if (shouldHandleOtp && isOtpError(json.error)) {
+        if (twoFactorState.token !== null) {
+          // the current otp code must be invalid since it errored
+          twoFactorState.token = null;
+        }
+        // just in case this isn't already true
+        twoFactorState.isRequired = Promise.resolve(true);
+        return internalPublish(pkgName, opts, twoFactorState);
       }
-      // just in case this isn't already true
-      twoFactorState.isRequired = Promise.resolve(true);
-      return internalPublish(pkgName, opts, twoFactorState);
+      error(
+        `an error occurred while publishing ${pkgName}: ${json.error.code}`,
+        json.error.summary,
+        json.error.detail ? "\n" + json.error.detail : ""
+      );
+      return { published: false };
     }
-    error(
-      `an error occurred while publishing ${pkgName}: ${json.error.code}`,
-      json.error.summary,
-      json.error.detail ? "\n" + json.error.detail : ""
-    );
-    return { published: false };
   }
+
   return { published: true };
 }
 

packages/cli/src/utils/getLastJsonObjectFromString.test.ts

@@ -0,0 +1,70 @@
+import { getLastJsonObjectFromString } from "./getLastJsonObjectFromString";
+
+describe("getLastJsonObjectFromString", () => {
+  it("should handle stringified object", () => {
+    expect(
+      getLastJsonObjectFromString(JSON.stringify({ test: "foo" }))
+    ).toEqual({ test: "foo" });
+  });
+
+  it("should handle stringified deep object", () => {
+    expect(
+      getLastJsonObjectFromString(
+        JSON.stringify({
+          test: "foo",
+          bar: { baz: { qwe: "rty" }, arr: [1, 2, 3, 4] }
+        })
+      )
+    ).toEqual({
+      test: "foo",
+      bar: { baz: { qwe: "rty" }, arr: [1, 2, 3, 4] }
+    });
+  });
+
+  it("should handle leading whitespace", () => {
+    expect(
+      getLastJsonObjectFromString(
+        `   \n\n  ${JSON.stringify({ test: "foo", baz: { qwe: "rty" } })}`
+      )
+    ).toEqual({ test: "foo", baz: { qwe: "rty" } });
+  });
+
+  it("should handle trailing whitespace", () => {
+    expect(
+      getLastJsonObjectFromString(
+        `${JSON.stringify({ test: "foo", baz: { qwe: "rty" } })}   \n\n  `
+      )
+    ).toEqual({ test: "foo", baz: { qwe: "rty" } });
+  });
+
+  it("should handle trailing text", () => {
+    expect(
+      getLastJsonObjectFromString(
+        `${JSON.stringify({ test: "foo", baz: { qwe: "rty" } })}   \n\n  test`
+      )
+    ).toEqual({ test: "foo", baz: { qwe: "rty" } });
+  });
+
+  it("should handle string with multiple objects", () => {
+    expect(
+      getLastJsonObjectFromString(
+        `${JSON.stringify({
+          test: "foo",
+          baz: { qwe: "rty" }
+        })}   \n\n  ${JSON.stringify({ much: "awesome" })}`
+      )
+    ).toEqual({ much: "awesome" });
+  });
+
+  it("should return `null` for an empty string", () => {
+    expect(getLastJsonObjectFromString("")).toEqual(null);
+  });
+
+  it("should return `null` for a string with a broken object", () => {
+    expect(getLastJsonObjectFromString(`{"bar:"`)).toEqual(null);
+  });
+
+  it("should return `null` for a string without an object", () => {
+    expect(getLastJsonObjectFromString(`qwerty`)).toEqual(null);
+  });
+});

packages/cli/src/utils/getLastJsonObjectFromString.ts

@@ -0,0 +1,15 @@
+export const getLastJsonObjectFromString = (str: string) => {
+  str = str.replace(/[^}]*$/, "");
+
+  while (str) {
+    str = str.replace(/[^{]*/, "");
+
+    try {
+      return JSON.parse(str);
+    } catch (err) {
+      // move past the potentially leading `{` so the regexp in the loop can try to match for the next `{`
+      str = str.slice(1);
+    }
+  }
+  return null;
+};