Auto-detect esbuild for v13+ branches#113
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds automatic esbuild enablement for newer Highcharts branches (v13+) by probing the branch for the presence of ts/masters/highcharts-autoload.src.ts, and updates tests/docs to reflect the new behavior.
Changes:
- Add
branchNeedsEsbuild()detection (GitHubHEADrequest) and auto-enable esbuild inhandlerDefaultwhen appropriate. - Tighten URL SHA-rewrite logic to replace only full path segments (avoid filename corruption) and fix single-segment branch rewrite behavior.
- Add/adjust unit + Hurl integration tests and update documentation; bump
@highcharts/highcharts-utils.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| app/handlers.js | Adds esbuild auto-detection + caching and updates URL rewriting before serving/building. |
| README.md | Documents automatic esbuild behavior for v13+ and how to verify via headers. |
| test/esbuild.js | Extends unit coverage for autoload + data-sorting paths/configs. |
| test/hurl/smoke.hurl | Adds basic local smoke check. |
| test/hurl/esbuild-autoload.hurl | Adds integration checks for esbuild compilation + auto-detection. |
| test/hurl/dashboards.hurl | Adds integration checks for dashboards endpoints. |
| test.hurl | Points users to the new test/hurl/*.hurl layout. |
| package.json | Pins @highcharts/highcharts-utils to a GitHub branch. |
| package-lock.json | Updates lockfile for dependency changes. |
| const esbuildCache = new Map() | ||
| const ESBUILD_DETECTION_FILE = 'ts/masters/highcharts-autoload.src.ts' |
There was a problem hiding this comment.
esbuildCache is an unbounded Map and will grow indefinitely with unique refs/SHAs, which can become a long-lived memory leak in a server process. Consider switching to a bounded LRU/TTL cache (e.g., cap entries and/or expire after N minutes), or only caching for known branch names (not every commit SHA).
| }, | ||
| "devDependencies": { | ||
| "@highcharts/highcharts-utils": "github:highcharts/highcharts-utils", | ||
| "@highcharts/highcharts-utils": "github:highcharts/highcharts-utils#fix/uniquekey-compile-on-demand", |
There was a problem hiding this comment.
Pinning @highcharts/highcharts-utils to a moving branch (#fix/uniquekey-compile-on-demand) makes installs non-reproducible and can unexpectedly change behavior over time. Consider pinning to a specific commit SHA or a tagged release, and documenting why this fork/branch is required.
| "@highcharts/highcharts-utils": "github:highcharts/highcharts-utils#fix/uniquekey-compile-on-demand", | |
| "@highcharts/highcharts-utils": "github:highcharts/highcharts-utils#<full-commit-sha-for-the-required-fix>", |
Summary
Automatically detect and enable esbuild mode for Highcharts v13+ branches by checking for the presence of
ts/masters/highcharts-autoload.src.tson the branch.Changes
branchNeedsEsbuild()function that performs a GitHub HEAD request to detect v13+ branches.replace()calls to path segment boundaries to prevent filename corruptionrespondToClientfor consistency@highcharts/highcharts-utilsdependency