ci: typecheck github-script code by philiptaron · Pull Request #485846 · NixOS/nixpkgs

philiptaron · 2026-02-01T05:29:38Z

Summary

Add TypeScript type checking for ci/github-script/ JavaScript files to catch bugs at development time rather than in production.

Motivation

While reviewing PR #456481, I found several bugs that TypeScript's --checkJs mode catches automatically:

Set.length instead of Set.size (silent failure)
Undeclared loop variables (implicit globals)
Missing required function parameters
Incorrect API usage

Changes

Infrastructure (commit 1):

Add jsconfig.json with checkJs: true
Add typescript and @types/node as devDependencies

Bug fixes (commits 2-8):

commits.js: Add missing core parameter, declare loop variable
bot.js: Add missing workflowRunId to artifact downloads, fix Date arithmetic
reviews.js: Mark optional parameter as optional in JSDoc
merge.js: Add default values for optional parameters
withRateLimit.js: Suppress errors for untyped bottleneck package
supportedBranches.js: Replace deprecated module.parent

CI integration (commit 9):

Add typecheck-ci-scripts derivation in ci/github-script/default.nix
Expose via nix-build ci -A typecheck-ci-scripts
Add typecheck-ci-scripts command to repository's nix-shell
Update documentation in both ci/README.md and ci/github-script/README.md

I didn't wire this into any ran-by-CI workflow because I ran out of time. But having it here is a step in that direction.

Testing

Run nix-build ci -A typecheck-ci-scripts or nix-shell --run typecheck-ci-scripts.

Things done

Add jsconfig.json with checkJs enabled to catch common JavaScript bugs like using Set.length instead of Set.size, undeclared variables, and mismatched function signatures. Add typescript and @types/node as devDependencies. Document how to run the type checker in README.md.

The dismissReviews function requires core for logging warnings, but the call site was missing this parameter. Found by TypeScript.

The for-of loop was using an undeclared variable 'line', which would create an implicit global. Add const declaration. Found by TypeScript.

Add missing workflowRunId parameter to all downloadArtifact calls. The @actions/artifact API requires this field in the findBy object. Also fix Date arithmetic by calling .getTime() - TypeScript correctly flags that Date objects can't be subtracted from numbers without explicit conversion. Found by TypeScript.

The postReview function has a default value for event, so the JSDoc type should mark it as optional with ?. Found by TypeScript.

The runChecklist function is intentionally called without user and userIsMaintainer when checking label eligibility (vs actual merging). Add default values to make this explicit. Found by TypeScript.

@ts-expect-error

The bottleneck npm package doesn't have TypeScript type definitions. Add @ts-expect-error comments to suppress the "not constructable" errors while still enabling type checking for the rest of the file.

module.parent has been deprecated since Node.js 14. Use the modern require.main === module pattern instead. Found by TypeScript.

Add a Nix derivation that runs TypeScript type checking on the ci/github-script JavaScript files. This ensures type errors are caught in CI, not just locally. The check is defined in ci/github-script/default.nix and exposed via: - `nix-build ci -A typecheck-ci-scripts` - `typecheck-ci-scripts` command in the development shell (via passthru.driver)

mdaniels5757

This looks great! A couple of minor things:

One issue (which is mostly pre-existing, but made a bit worse here): nodejs is v24 on master (and v24 is used by actions/github-script). But nodejs is v22 on stable, and the @types/node version added in this PR is v22 as well. I think pinning to v24 would be prudent, given that's what is used by actions/github-script.

In addition to the suggestions below, there's one more place to pin nodejs to 24 in: ci/github-script/shell.nix (which is untouched by this PR).

Also in ci/github-script/shell.nix, it would also be nice to add (pkgs.callPackage ./. { }).passthru.driver to packages, so typecheck-ci-scripts can be run directly from the shell without extra nix-shell/nix-build invocations. (The documentation could then be updated accordingly.)

mdaniels5757 · 2026-02-01T14:50:42Z

ci/github-script/package.json

    "commander": "14.0.0"
+  },
+  "devDependencies": {
+    "@types/node": "^22.0.0",


Node 24 is used by the github-script action, and is the current version of nodejs on master.

package-lock.json will also need to be updated, of course.

Suggested change

"@types/node": "^22.0.0",

"@types/node": "^24.0.0",

mdaniels5757 · 2026-02-01T14:52:39Z

ci/github-script/default.nix

+# Type check the CI scripts in ci/github-script using TypeScript
+{
+  importNpmLock,
+  nodejs,


nodejs on master is v24, but v22 on stable.

Suggested change

nodejs,

nodejs_24,

mdaniels5757 · 2026-02-01T14:52:56Z

ci/github-script/default.nix

+let
+  npmDeps = importNpmLock.buildNodeModules {
+    npmRoot = ./.;
+    inherit nodejs;


Suggested change

inherit nodejs;

nodejs = nodejs_24;

mdaniels5757 · 2026-02-01T14:53:06Z

ci/github-script/default.nix

+in
+runCommand "typecheck-ci-scripts"
+  {
+    nativeBuildInputs = [ nodejs ];


Suggested change

nativeBuildInputs = [ nodejs ];

nativeBuildInputs = [ nodejs_24 ];

mdaniels5757 · 2026-02-01T14:54:16Z

ci/supportedBranches.js

@@ -1,3 +1,3 @@
 #!/usr/bin/env nix-shell
 /*
 #!nix-shell -i node -p nodejs


Suggested change

#!nix-shell -i node -p nodejs_24

Seems like this shebang isn't necessary anymore since it's always imported directly from node

Yes, I think I should delete this -- but instead of doing the "nix-build to run the tool thing that I have done, instead use nix-shell in the shebang to have the tool be runnable outside of Nix.

mdaniels5757 · 2026-02-01T15:14:50Z

ci/github-script/jsconfig.json

+    "allowJs": true,
+    "noEmit": true,
+    "target": "ES2024",
+    "lib": ["ESNext"],


Per https://github.com/tsconfig/bases/blob/main/bases/node24.json:

Suggested change

"lib": ["ESNext"],

"lib": [

"es2024",

"ESNext.Array",

"ESNext.Collection",

"ESNext.Error",

"ESNext.Iterator",

"ESNext.Promise"

],

infinisil

LGTM and tested (the type checks even work with my neovim), thank you!

I agree with @mdaniels5757 that it would be good to pin this to node 24 (even better would be if GitHub actions would use the node version from Nix, but oh well)

infinisil · 2026-02-03T14:14:11Z

ci/supportedBranches.js

@@ -1,3 +1,3 @@
 #!/usr/bin/env nix-shell
 /*
 #!nix-shell -i node -p nodejs


Seems like this shebang isn't necessary anymore since it's always imported directly from node

philiptaron · 2026-02-03T16:01:26Z

Thanks for the reviews @infinisil and @mdaniels5757 -- will push an updated version sometime today hopefully. I plan on including a CI step gated on changes to these files so that it actually will mark the CI red if there are errors in typechecking. 🎉

philiptaron added 9 commits January 31, 2026 20:15

ci/github-script/commits: pass core to dismissReviews

f0b8466

The dismissReviews function requires core for logging warnings, but the call site was missing this parameter. Found by TypeScript.

ci/github-script/commits: declare loop variable

91edfab

The for-of loop was using an undeclared variable 'line', which would create an implicit global. Add const declaration. Found by TypeScript.

ci/github-script/reviews: make event parameter optional

e6b4792

The postReview function has a default value for event, so the JSDoc type should mark it as optional with ?. Found by TypeScript.

ci/github-script/merge: add defaults for optional parameters

c26c785

The runChecklist function is intentionally called without user and userIsMaintainer when checking label eligibility (vs actual merging). Add default values to make this explicit. Found by TypeScript.

ci/github-script/withRateLimit: suppress Bottleneck type errors

1227412

The bottleneck npm package doesn't have TypeScript type definitions. Add @ts-expect-error comments to suppress the "not constructable" errors while still enabling type checking for the rest of the file.

ci/supportedBranches: replace deprecated module.parent

3972dae

module.parent has been deprecated since Node.js 14. Use the modern require.main === module pattern instead. Found by TypeScript.

philiptaron requested a review from a team February 1, 2026 05:29

nixpkgs-ci bot requested review from LeSuisse, MattSturgeon, Mic92, infinisil, mdaniels5757, mweinelt, risicle and zowoq February 1, 2026 05:32

mdaniels5757 approved these changes Feb 1, 2026

View reviewed changes

nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Feb 1, 2026

infinisil approved these changes Feb 3, 2026

View reviewed changes

nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Feb 3, 2026

infinisil mentioned this pull request Feb 7, 2026

ci: Fix team review requests from the bot #488000

Merged

2 tasks

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Feb 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

ci: typecheck github-script code#485846

ci: typecheck github-script code#485846
philiptaron wants to merge 9 commits intoNixOS:masterfrom
philiptaron:type-check-ci-code

philiptaron commented Feb 1, 2026 •

edited

Loading

Uh oh!

mdaniels5757 left a comment

Uh oh!

mdaniels5757 Feb 1, 2026

Uh oh!

mdaniels5757 Feb 1, 2026

Uh oh!

mdaniels5757 Feb 1, 2026

Uh oh!

mdaniels5757 Feb 1, 2026

Uh oh!

mdaniels5757 Feb 1, 2026

Uh oh!

infinisil Feb 3, 2026

Uh oh!

philiptaron Feb 3, 2026

Uh oh!

mdaniels5757 Feb 1, 2026

Uh oh!

infinisil left a comment •

edited

Loading

Uh oh!

infinisil Feb 3, 2026

Uh oh!

philiptaron commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	nativeBuildInputs = [ nodejs ];
	nativeBuildInputs = [ nodejs_24 ];

-    "lib": ["ESNext"],
+    "lib": [
+      "es2024",
+      "ESNext.Array",
+      "ESNext.Collection",
+      "ESNext.Error",
+      "ESNext.Iterator",
+      "ESNext.Promise"
+    ],

Uh oh!

Conversation

philiptaron commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Changes

Testing

Things done

Uh oh!

mdaniels5757 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

infinisil left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

philiptaron commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

philiptaron commented Feb 1, 2026 •

edited

Loading

infinisil left a comment •

edited

Loading