Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prepublish checks to block releases of non-snapshot versionsfirst one #2037

Merged
merged 5 commits into from
Dec 19, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .changeset/lemon-maps-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
3 changes: 2 additions & 1 deletion packages/kas/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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)'"
},
Expand All @@ -43,4 +44,4 @@
"algebra",
"symbolic"
]
}
}
3 changes: 2 additions & 1 deletion packages/keypad-context/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -34,4 +35,4 @@
"react": "^18.2.0"
},
"keywords": []
}
}
3 changes: 2 additions & 1 deletion packages/kmath/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -34,4 +35,4 @@
"underscore": "1.4.4"
},
"keywords": []
}
}
6 changes: 4 additions & 2 deletions packages/math-input/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -78,4 +80,4 @@
"react-transition-group": "^4.4.1"
},
"keywords": []
}
}
3 changes: 2 additions & 1 deletion packages/perseus-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {},
Expand All @@ -30,4 +31,4 @@
},
"peerDependencies": {},
"keywords": []
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OCD-level 10 comment: looks like all the package.json lost their newline at end of file. I'm not sure if that matters, but Github flags it so... 🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make a difference. I just saved like usual - no idea why this means it doesn't get a newline.

3 changes: 2 additions & 1 deletion packages/perseus-editor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -99,4 +100,4 @@
"underscore": "^1.4.4"
},
"keywords": []
}
}
3 changes: 2 additions & 1 deletion packages/perseus-linter/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -35,4 +36,4 @@
"prop-types": "15.6.1"
},
"keywords": []
}
}
3 changes: 2 additions & 1 deletion packages/perseus/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down Expand Up @@ -124,4 +125,4 @@
"underscore": "^1.4.4"
},
"keywords": []
}
}
3 changes: 2 additions & 1 deletion packages/pure-markdown/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -31,4 +32,4 @@
"devDependencies": {},
"peerDependencies": {},
"keywords": []
}
}
3 changes: 2 additions & 1 deletion packages/simple-markdown/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -37,4 +38,4 @@
"keywords": [
"markdown"
]
}
}
62 changes: 40 additions & 22 deletions utils/internal/pre-publish-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,47 @@
* Pre-publish utilities to verify that our publish will go smoothly.
*/

const checkPublishConfig = ({name, publishConfig, private: isPrivate}) => {
const checkPublishConfig = ({
name,
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";

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
if (isPrivate && publishConfig.access !== "restricted") {
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
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".`,
);
returnCode = false;
}
Comment on lines +31 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay. Thanks for adding this check here too!

Comment on lines +31 to +40
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: New check to make sure all the packages we publish are running our prepublish check to avoid snapshot releases usurping real releases.

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(
Expand All @@ -32,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) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I just changed that nested if in an else to an else if.

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.`,
Expand All @@ -64,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};
7 changes: 7 additions & 0 deletions utils/package-pre-publish-check.sh
Original file line number Diff line number Diff line change
@@ -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
11 changes: 7 additions & 4 deletions utils/pre-publish-check-ci.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
},
Expand Down
8 changes: 8 additions & 0 deletions utils/publish-snapshot.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading