Bundle Actions using esbuild by henrymercer · Pull Request #3054 · github/codeql-action · GitHub

This PR bundles the Action using esbuild.

Reasoning

Avoid risks of long paths in node_modules, specifically the risk that these paths could exceed MAX_PATH on Windows when checked out.
Speed up the download of the Action, and every code scanning workflow as a result.

Downsides we should be aware of

Potential for more merge conflicts
- Mitigation: I haven't enabled minification which I hope will reduce the likelihood of merge conflicts
- Mitigation: You can now run the rebuild workflow on PRs even if they have merge conflicts (Enable rebuilding PRs with conflicts #3014)

Risk assessment

For internal use only. Please select the risk level of this change:

High risk: Changes are not fully under feature flags, have limited visibility and/or cannot be tested outside of production.

This is a risky change. Things I've done to minimise the risk:

Avoid changing the module resolution system — this is tricky and requires us to change a lot of our unit tests.
Keep the compiled Actions in the same place, so that users invoking them directly do not need to change their paths.
Keep defaults.json copied over to lib.
Minimise changes to unit testing setup by preserving the "compile to JS" workflow that's known to work reasonably well with ava.

Merge / deployment checklist

Confirm this change is backwards compatible with existing workflows.
Consider adding a changelog entry for this change.
Confirm the readme and docs have been updated if necessary.

It isn't working, and this PR didn't break it. Let's fix it in a separate PR.

This doesn't give us much net new test coverage

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Pull Request Overview

This PR introduces bundling of the CodeQL Action using esbuild to replace the previous TypeScript compilation approach. The change aims to reduce download times, speed up workflows, and avoid Windows path length issues by consolidating dependencies into fewer files.

Key Changes:

Migration from TypeScript compilation to esbuild bundling for JavaScript generation
Complete replacement of all compiled JavaScript files in the lib directory
Preservation of the existing module structure and functionality while optimizing the build output

Reviewed Changes

Copilot reviewed 63 out of 23839 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
lib/setup-codeql.test.js	Updated bundled test file for CodeQL setup functionality
lib/setup-codeql.js	Updated bundled implementation file for CodeQL setup and download logic
lib/resolve-environment.js	Updated bundled utility for build environment resolution
lib/repository.js	Updated bundled repository utilities for GitHub repository handling
lib/overlay-database-utils.test.js	Updated bundled test file for overlay database functionality
lib/overlay-database-utils.js	Updated bundled implementation for overlay database caching and management
lib/logging.js	Updated bundled logging utilities for Actions and runner logging
lib/languages.js	Updated bundled language enumeration definitions
lib/init.test.js	Updated bundled test file for initialization functionality
lib/init.js	Updated bundled initialization logic for CodeQL setup
lib/init-action-post-helper.test.js	Updated bundled test file for post-action helper functionality
lib/init-action-post-helper.js	Updated bundled post-action cleanup and SARIF upload logic
lib/git-utils.test.js	Updated bundled test file for Git utilities
lib/git-utils.js	Updated bundled Git operation utilities
lib/fingerprints.test.js	Updated bundled test file for SARIF fingerprinting
lib/fingerprints.js	Updated bundled SARIF fingerprinting implementation
lib/feature-flags.test.js	Updated bundled test file for feature flag functionality

Nice. I haven't tried this locally, but it looks good.

If I have time later, I'll try checking this PR out and making sure the dev flow still works for me.

Let's use npm ci instead of npm install in the workflows.

We'll need to update the set of required checks before merging this PR.

Let's run npm ci instead of npm install. Also don't forget to update poackage-lock.json`

Avoid changing the module resolution system — this is tricky and requires us to change a lot of our unit tests.

Note that bundling CJS is less reliable than ESM due to its dynamic nature. ESBuild is designed with ESM in mind, and there has been (at least historically) some gotchas wrt. its CJS bundling.

Note that bundling CJS is less reliable than ESM due to its dynamic nature. ESBuild is designed with ESM in mind, and there has been (at least historically) some gotchas wrt. its CJS bundling.

Is there a different bundler you'd recommend for CJS? I chose ESBuild since it's much faster than webpack and we're using it successfully in other projects, but we could explore something else.

Note that bundling CJS is less reliable than ESM due to its dynamic nature. ESBuild is designed with ESM in mind, and there has been (at least historically) some gotchas wrt. its CJS bundling.

Is there a different bundler you'd recommend for CJS? I chose ESBuild since it's much faster than webpack and we're using it successfully in other projects, but we could explore something else.

It looks like the typescript-action template repo uses rollup, no idea if it is better. https://github.com/actions/typescript-action/blob/main/package.json#L33 . They used to use ncc but switched not to long ago to rollup: actions/typescript-action#969

It looks like the typescript-action template repo uses rollup, no idea if it is better. https://github.com/actions/typescript-action/blob/main/package.json#L33 . They used to use ncc but switched not to long ago to rollup: actions/typescript-action#969

I believe rollup was designed for ES modules perhaps more so than esbuild — you need to use a plugin to use it with CommonJS modules.

It looks like the typescript-action template repo uses rollup, no idea if it is better. actions/typescript-action@main/package.json#L33 . They used to use ncc but switched not to long ago to rollup: actions/typescript-action#969

I believe rollup was designed for ES modules perhaps more so than esbuild — you need to use a plugin to use it with CommonJS modules.

esbuild has been my favourite tool for bundling. It's easy to use, still fairly flexible, and fast. I have only used it for ESM bundling, though. I know that vite has used esbuild for transpilation and rollup for bundling (not sure if still true). Historically webpack has had the best support for CJS bundling, but it is slow and bloated. I haven't kept up on the bundling scene the past 2 years though, so a lot may have happened...

This means we avoid doing a type checking pass twice

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

The esbuild setup looks sound. You will likely want 👀 from somebody else as well as I don't know of the complexities around distributing codeql-action.

As I said, there can be nasty surprises when bundling CJS, so be sure to test various setups.

It's no longer needed since we aren't checking in `node_modules`.

Looks good to me and I am happy with the testing we have done to believe that this doesn't break anything horribly.

There might be minor follow-up changes needed to edit documentation or some automation, but I think it's fine to ship-to-learn a change like this as long as we are happy that it doesn't break the actual functionality of the Action.

henrymercer added 12 commits August 21, 2025 13:38

Bundle Actions using esbuild

fee3663

Compile sources to separate directory for tests

67d4718

Remove node_modules directory

8b7a4ec

Check in compiled files

88d8b86

Use "Rebuild" workflow instead of "Update dependencies"

6d34e4e

Install dependencies in PR checks

31ee7f5

Run more checks on push

06f83b7

Combine basic jobs to reduce Actions usage

3edad3e

Cache npm dependencies

a2df83b

Don't run linter in CI on Windows

d8905c2

It isn't working, and this PR didn't break it. Let's fix it in a separate PR.

Merge branch 'main' into henrymercer/bundle

f04e228

Copy defaults.json to lib

3e725de

github-advanced-security Bot found potential problems Aug 21, 2025

View reviewed changes

Comment thread build.mjs Fixed

henrymercer and others added 5 commits August 21, 2025 16:19

Install dependencies in code scanning config test

f8c0d73

Install npm dependencies where necessary in generated workflows

879c788

Install dependencies in query filters tests

88f7d3d

Delete expected queries runs check

36468ba

This doesn't give us much net new test coverage

Remove unused import in build script

786d3aa

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

henrymercer marked this pull request as ready for review August 21, 2025 15:47

henrymercer requested a review from a team as a code owner August 21, 2025 15:47

henrymercer requested review from Copilot and removed request for a team August 21, 2025 15:47

Copilot AI reviewed Aug 21, 2025

View reviewed changes

Comment thread build.mjs Outdated

aeisenberg reviewed Aug 21, 2025

View reviewed changes

Comment thread .github/workflows/pr-checks.yml

aibaars reviewed Aug 21, 2025

View reviewed changes

Comment thread .github/workflows/codescanning-config-cli.yml Outdated

aibaars reviewed Aug 21, 2025

View reviewed changes

henrymercer requested a review from aibaars August 21, 2025 17:48

Use npm ci in CI

4da503e

aibaars reviewed Aug 21, 2025

View reviewed changes

navntoft reviewed Aug 22, 2025

View reviewed changes

Comment thread build.mjs

Comment thread build.mjs Outdated

henrymercer added 3 commits August 22, 2025 12:31

Transpile separately in both build and test steps

8a3bfe6

This means we avoid doing a type checking pass twice

Implement clean and copy as custom plugins

e274fb8

Specify esbuild target

ca5a69b

henrymercer requested a review from navntoft August 22, 2025 12:06

github-advanced-security Bot found potential problems Aug 22, 2025

View reviewed changes

Comment thread build.mjs Fixed

Avoid automatic semicolon insertion

d7c1640

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

navntoft previously approved these changes Aug 22, 2025

View reviewed changes

Check in precompiled upload-lib.ts for testing environments

d2dfa8c

henrymercer dismissed navntoft’s stale review via d2dfa8c August 27, 2025 09:59

henrymercer added 4 commits August 29, 2025 12:08

Merge branch 'main' into henrymercer/bundle

68be0dc

Bump minor version number

6d7d2c6

Update CONTRIBUTING.md

138655e

Remove removeNPMAbsolutePaths

3e493e7

It's no longer needed since we aren't checking in `node_modules`.

mbg approved these changes Aug 29, 2025

View reviewed changes

henrymercer merged commit 02ab253 into main Aug 29, 2025

henrymercer deleted the henrymercer/bundle branch August 29, 2025 13:02

This was referenced Aug 29, 2025

Remove node_modules and compile typescript into minified scripts #2578

Closed

Action takes 5-6 seconds to download #2542

Closed

github-actions Bot mentioned this pull request Sep 1, 2025

Merge main into releases/v3 #3067

Merged

8 tasks

Copilot Bot mentioned this pull request Sep 2, 2025

Add retroactive changelog note for #3054 #3078

Merged

henrymercer mentioned this pull request Oct 10, 2025

Revert "Rebuild" commit rather than "Update dependencies" #3200

Merged

Bundle Actions using esbuild by henrymercer · Pull Request #3054 · github/codeql-action · GitHub

Uh oh!

Bundle Actions using esbuild - #3054

Bundle Actions using esbuild#3054henrymercer merged 27 commits intomaingithub/codeql-action:mainfrom henrymercer/bundlegithub/codeql-action:henrymercer/bundleCopy head branch name to clipboard

Conversation

henrymercer commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reasoning

Downsides we should be aware of

Risk assessment

Merge / deployment checklist

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

aeisenberg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aeisenberg commented Aug 21, 2025

Uh oh!

Uh oh!

aibaars left a comment

Choose a reason for hiding this comment

Uh oh!

henrymercer commented Aug 21, 2025

Uh oh!

aibaars left a comment

Choose a reason for hiding this comment

Uh oh!

navntoft left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

henrymercer commented Aug 22, 2025

Uh oh!

aibaars commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henrymercer commented Aug 22, 2025

Uh oh!

navntoft commented Aug 22, 2025

Uh oh!

Uh oh!

navntoft 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!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Bundle Actions using esbuild#3054
henrymercer merged 27 commits into
mainfrom
henrymercer/bundle

henrymercer commented Aug 21, 2025 •
edited
Loading

aibaars commented Aug 22, 2025 •
edited
Loading

navntoft left a comment •
edited
Loading