diff --git a/packages/cli/src/commands/publish/npm-utils.ts b/packages/cli/src/commands/publish/npm-utils.ts index 2db21d2ba..5f0c94670 100644 --- a/packages/cli/src/commands/publish/npm-utils.ts +++ b/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=")) + ); +}; + // 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="))) && - !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 }; } diff --git a/packages/cli/src/utils/getLastJsonObjectFromString.test.ts b/packages/cli/src/utils/getLastJsonObjectFromString.test.ts new file mode 100644 index 000000000..a0923baaa --- /dev/null +++ b/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); + }); +}); diff --git a/packages/cli/src/utils/getLastJsonObjectFromString.ts b/packages/cli/src/utils/getLastJsonObjectFromString.ts new file mode 100644 index 000000000..5328537ce --- /dev/null +++ b/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; +};