From 774926b8a48939e8b899a8d9b4474d4d6e381f19 Mon Sep 17 00:00:00 2001 From: Jeff Yates Date: Wed, 18 Dec 2024 17:54:57 -0600 Subject: [PATCH 1/5] Add prepublish checks to block releases of non-snapshot versions --- packages/kas/package.json | 3 ++- packages/keypad-context/package.json | 3 ++- packages/kmath/package.json | 3 ++- packages/math-input/package.json | 6 ++++-- packages/perseus-core/package.json | 3 ++- packages/perseus-editor/package.json | 3 ++- packages/perseus-linter/package.json | 3 ++- packages/perseus/package.json | 3 ++- packages/pure-markdown/package.json | 3 ++- packages/simple-markdown/package.json | 3 ++- utils/internal/pre-publish-utils.ts | 18 +++++++++++++++++- utils/package-pre-publish-check.sh | 7 +++++++ utils/publish-snapshot.sh | 8 ++++++++ 13 files changed, 54 insertions(+), 12 deletions(-) create mode 100755 utils/package-pre-publish-check.sh diff --git a/packages/kas/package.json b/packages/kas/package.json index b25c05d87d..2695f8a564 100644 --- a/packages/kas/package.json +++ b/packages/kas/package.json @@ -22,6 +22,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "gen:parsers": "node src/parser-generator.ts", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, @@ -43,4 +44,4 @@ "algebra", "symbolic" ] -} +} \ No newline at end of file diff --git a/packages/keypad-context/package.json b/packages/keypad-context/package.json index a5b3a8ab46..29b3a9476c 100644 --- a/packages/keypad-context/package.json +++ b/packages/keypad-context/package.json @@ -22,6 +22,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -34,4 +35,4 @@ "react": "^18.2.0" }, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/kmath/package.json b/packages/kmath/package.json index 23bcbfbdb9..7c65b5891f 100644 --- a/packages/kmath/package.json +++ b/packages/kmath/package.json @@ -21,6 +21,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -34,4 +35,4 @@ "underscore": "1.4.4" }, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/math-input/package.json b/packages/math-input/package.json index 762156a0cb..1a0a8c1937 100644 --- a/packages/math-input/package.json +++ b/packages/math-input/package.json @@ -36,7 +36,9 @@ "files": [ "dist" ], - "scripts": {}, + "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh" + }, "dependencies": { "@khanacademy/keypad-context": "^1.0.9", "@khanacademy/perseus-core": "3.0.3", @@ -78,4 +80,4 @@ "react-transition-group": "^4.4.1" }, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/perseus-core/package.json b/packages/perseus-core/package.json index b3ae2af574..f7d157bd32 100644 --- a/packages/perseus-core/package.json +++ b/packages/perseus-core/package.json @@ -22,6 +22,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": {}, @@ -30,4 +31,4 @@ }, "peerDependencies": {}, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/perseus-editor/package.json b/packages/perseus-editor/package.json index db6db653da..4611636d5d 100644 --- a/packages/perseus-editor/package.json +++ b/packages/perseus-editor/package.json @@ -31,6 +31,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -99,4 +100,4 @@ "underscore": "^1.4.4" }, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/perseus-linter/package.json b/packages/perseus-linter/package.json index 9b9f4734dd..25c91ba406 100644 --- a/packages/perseus-linter/package.json +++ b/packages/perseus-linter/package.json @@ -22,6 +22,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -35,4 +36,4 @@ "prop-types": "15.6.1" }, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/perseus/package.json b/packages/perseus/package.json index b2bc22905d..ba5ebf7d83 100644 --- a/packages/perseus/package.json +++ b/packages/perseus/package.json @@ -37,6 +37,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -124,4 +125,4 @@ "underscore": "^1.4.4" }, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/pure-markdown/package.json b/packages/pure-markdown/package.json index 18e57dfde8..0bc17dc97a 100644 --- a/packages/pure-markdown/package.json +++ b/packages/pure-markdown/package.json @@ -22,6 +22,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -31,4 +32,4 @@ "devDependencies": {}, "peerDependencies": {}, "keywords": [] -} +} \ No newline at end of file diff --git a/packages/simple-markdown/package.json b/packages/simple-markdown/package.json index 50545537a0..91003ec1b5 100644 --- a/packages/simple-markdown/package.json +++ b/packages/simple-markdown/package.json @@ -22,6 +22,7 @@ "dist" ], "scripts": { + "prepublishOnly": "../../utils/package-pre-publish-check.sh", "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'" }, "dependencies": { @@ -37,4 +38,4 @@ "keywords": [ "markdown" ] -} +} \ No newline at end of file diff --git a/utils/internal/pre-publish-utils.ts b/utils/internal/pre-publish-utils.ts index 991baeac45..92f4e57f6b 100644 --- a/utils/internal/pre-publish-utils.ts +++ b/utils/internal/pre-publish-utils.ts @@ -2,7 +2,12 @@ * Pre-publish utilities to verify that our publish will go smoothly. */ -const checkPublishConfig = ({name, publishConfig, private: isPrivate}) => { +const checkPublishConfig = ({ + name, + publishConfig, + private: isPrivate, + scripts, +}) => { // first check if is marked as public and there's access to publish the current package if (!publishConfig || (!isPrivate && publishConfig.access !== "public")) { const requiredAccessType = isPrivate ? "restricted" : "public"; @@ -20,6 +25,17 @@ const checkPublishConfig = ({name, publishConfig, private: isPrivate}) => { ); process.exit(1); } + + // check that we are running our pre-publish check for this package + if ( + !scripts.prepublishOnly || + !scripts.prepublishOnly.includes("utils/package-pre-publish-check.sh") + ) { + console.error( + `ERROR: ${name} must have a "prepublishOnly" script that runs "utils/package-pre-publish-check.sh".`, + ); + process.exit(1); + } }; const checkField = (pkgJson, field, value) => { diff --git a/utils/package-pre-publish-check.sh b/utils/package-pre-publish-check.sh new file mode 100755 index 0000000000..3dab0ddde8 --- /dev/null +++ b/utils/package-pre-publish-check.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash + +# Check if SNAPSHOT_RELEASE is set and the version does not start with 0.0.0-PR +if [ "$SNAPSHOT_RELEASE" = "1" ] && ! [[ "$npm_package_version" =~ ^0\.0\.0-PR ]]; then + echo "Error: Snapshot publish attempted, but $npm_package_name@$npm_package_version does not match version scheme for snapshots. Publish disallowed." + exit 1 +fi diff --git a/utils/publish-snapshot.sh b/utils/publish-snapshot.sh index b64c8be2b5..d683670897 100755 --- a/utils/publish-snapshot.sh +++ b/utils/publish-snapshot.sh @@ -18,6 +18,14 @@ MYPATH=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd ) # ROOT is the root directory of our project. ROOT="$MYPATH/.." +# This is used in prepublishOnly hooks to verify that the package is correctly +# versioned for a snapshot release before proceeding. +# This is done to catch a race condition where a main release is occurring +# while a snapshot release is requested, avoiding us publishing packages +# that we shouldn't be. +# See https://khanacademy.atlassian.net/wiki/spaces/ENG/pages/3571646568/Race+condition+breaks+Perseus+release +SNAPSHOT_RELEASE=1 + pushd "$ROOT" verify_env() { From 41821570c3f97355798035987697cfa8449d1999 Mon Sep 17 00:00:00 2001 From: Jeff Yates Date: Wed, 18 Dec 2024 18:00:53 -0600 Subject: [PATCH 2/5] Make prepublish checks run on all things and report, instead of failing on first one --- utils/internal/pre-publish-utils.ts | 48 +++++++++++++++-------------- utils/pre-publish-check-ci.ts | 11 ++++--- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/utils/internal/pre-publish-utils.ts b/utils/internal/pre-publish-utils.ts index 92f4e57f6b..08e12e6e84 100644 --- a/utils/internal/pre-publish-utils.ts +++ b/utils/internal/pre-publish-utils.ts @@ -7,7 +7,9 @@ const checkPublishConfig = ({ publishConfig, private: isPrivate, scripts, -}) => { +}): boolean => { + let returnCode = true; + // first check if is marked as public and there's access to publish the current package if (!publishConfig || (!isPrivate && publishConfig.access !== "public")) { const requiredAccessType = isPrivate ? "restricted" : "public"; @@ -15,7 +17,7 @@ const checkPublishConfig = ({ console.error( `ERROR: ${name} is missing a "publishConfig": {"access": "${requiredAccessType}"} section.`, ); - process.exit(1); + returnCode = false; } // also check if is marked as private and there's restricted access defined @@ -23,7 +25,7 @@ const checkPublishConfig = ({ console.error( `ERROR: ${name} is marked as private but there is a "publishConfig": {"access": "public"} section already defined. Please change it to "access": "restricted" or remove "private": true to make the package public.`, ); - process.exit(1); + returnCode = false; } // check that we are running our pre-publish check for this package @@ -34,11 +36,13 @@ const checkPublishConfig = ({ console.error( `ERROR: ${name} must have a "prepublishOnly" script that runs "utils/package-pre-publish-check.sh".`, ); - process.exit(1); + returnCode = false; } + return returnCode; }; -const checkField = (pkgJson, field, value) => { +const checkField = (pkgJson, field, value): boolean => { + let returnCode = true; if (Array.isArray(value)) { if (!value.includes(pkgJson[field])) { console.error( @@ -48,29 +52,29 @@ const checkField = (pkgJson, field, value) => { .map((value) => JSON.stringify(value)) .join(", ")}.`, ); - process.exit(1); - } - } else { - if (pkgJson[field] !== value) { - console.error( - `ERROR: ${ - pkgJson.name - } must have a "${field}" set to ${JSON.stringify(value)}.`, - ); - process.exit(1); + returnCode = false; } + } else if (pkgJson[field] !== value) { + console.error( + `ERROR: ${ + pkgJson.name + } must have a "${field}" set to ${JSON.stringify(value)}.`, + ); + returnCode = false; } + return returnCode; }; -const checkMain = (pkgJson) => checkField(pkgJson, "main", "dist/index.js"); +const checkMain = (pkgJson): boolean => + checkField(pkgJson, "main", "dist/index.js"); -const checkModule = (pkgJson) => +const checkModule = (pkgJson): boolean => checkField(pkgJson, "module", "dist/es/index.js"); -const checkSource = (pkgJson) => +const checkSource = (pkgJson): boolean => checkField(pkgJson, "source", ["src/index.js", "src/index.ts"]); -const checkPrivate = (pkgJson) => { +const checkPrivate = (pkgJson): boolean => { if (pkgJson.private) { console.warn( `${pkgJson.name} is private and won't be published to NPM.`, @@ -80,9 +84,7 @@ const checkPrivate = (pkgJson) => { return false; }; -const checkEntrypoints = (pkgJson) => { - checkModule(pkgJson); - checkMain(pkgJson); -}; +const checkEntrypoints = (pkgJson): boolean => + checkModule(pkgJson) && checkMain(pkgJson); export {checkPublishConfig, checkEntrypoints, checkSource, checkPrivate}; diff --git a/utils/pre-publish-check-ci.ts b/utils/pre-publish-check-ci.ts index 3b4f00c209..d8521179a1 100755 --- a/utils/pre-publish-check-ci.ts +++ b/utils/pre-publish-check-ci.ts @@ -21,10 +21,13 @@ fg(path.join(__dirname, "..", "packages", "*", "package.json")).then( // eslint-disable-next-line @typescript-eslint/no-require-imports const pkgJson = require(path.relative(__dirname, pkgPath)); - if (!checkPrivate(pkgJson)) { - checkPublishConfig(pkgJson); - checkEntrypoints(pkgJson); - checkSource(pkgJson); + if ( + !checkPrivate(pkgJson) && + !checkPublishConfig(pkgJson) && + !checkEntrypoints(pkgJson) && + !checkSource(pkgJson) + ) { + process.exit(1); } } }, From f721074e2810ae7558e180002fa1b635f63089e1 Mon Sep 17 00:00:00 2001 From: Jeff Yates Date: Wed, 18 Dec 2024 18:01:36 -0600 Subject: [PATCH 3/5] docs(changeset): --- .changeset/lemon-maps-explain.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changeset/lemon-maps-explain.md diff --git a/.changeset/lemon-maps-explain.md b/.changeset/lemon-maps-explain.md new file mode 100644 index 0000000000..a845151cc8 --- /dev/null +++ b/.changeset/lemon-maps-explain.md @@ -0,0 +1,2 @@ +--- +--- From 5dddee1e4a5d24b02a7a8b7b63a0e1f3d9743ee2 Mon Sep 17 00:00:00 2001 From: Jeff Yates Date: Wed, 18 Dec 2024 18:15:59 -0600 Subject: [PATCH 4/5] docs(changeset): Nothing has changed, but our action requires a changeset per package and I don't know how to do an infrastructure update like this and pass that check --- .changeset/rotten-squids-act.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/rotten-squids-act.md diff --git a/.changeset/rotten-squids-act.md b/.changeset/rotten-squids-act.md new file mode 100644 index 0000000000..a31bf99e52 --- /dev/null +++ b/.changeset/rotten-squids-act.md @@ -0,0 +1,14 @@ +--- +"@khanacademy/kas": patch +"@khanacademy/keypad-context": patch +"@khanacademy/kmath": patch +"@khanacademy/math-input": patch +"@khanacademy/perseus": patch +"@khanacademy/perseus-core": patch +"@khanacademy/perseus-editor": patch +"@khanacademy/perseus-linter": patch +"@khanacademy/pure-markdown": patch +"@khanacademy/simple-markdown": patch +--- + +Nothing has changed, but our action requires a changeset per package and I don't know how to do an infrastructure update like this and pass that check From 165f468d023e2f3019c754ff5906c843db3d46a6 Mon Sep 17 00:00:00 2001 From: Jeff Yates Date: Wed, 18 Dec 2024 18:25:29 -0600 Subject: [PATCH 5/5] Check everything before we quit checking --- utils/pre-publish-check-ci.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/utils/pre-publish-check-ci.ts b/utils/pre-publish-check-ci.ts index d8521179a1..5fd48cace1 100755 --- a/utils/pre-publish-check-ci.ts +++ b/utils/pre-publish-check-ci.ts @@ -16,6 +16,7 @@ import { // eslint-disable-next-line promise/catch-or-return fg(path.join(__dirname, "..", "packages", "*", "package.json")).then( (pkgPaths) => { + let allPassed = true; // eslint-disable-next-line promise/always-return for (const pkgPath of pkgPaths) { // eslint-disable-next-line @typescript-eslint/no-require-imports @@ -27,8 +28,13 @@ fg(path.join(__dirname, "..", "packages", "*", "package.json")).then( !checkEntrypoints(pkgJson) && !checkSource(pkgJson) ) { - process.exit(1); + allPassed = false; } } + + // Exit only after we've processed all the packages. + if (!allPassed) { + process.exit(1); + } }, );