Conversation
Use a distinct temp output file for each benchmark warmup iteration so repeated warmups do not reuse the same path on Windows. Add a regression test for runWarmup and keep benchmark temp-file cleanup aligned with the generated warmup outputs.
Reformat the mocked compressor setup in utils tests to match the repository's formatter style without changing behavior.
* main: Version Packages Version Packages
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
chore(deps): update dependency tsdown to ^0.21.0
fix(deps): update dependency oxc-minify to ^0.117.0
fix(deps): update dependency oxc-minify to ^0.119.0
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
fix(deps): update dependency oxc-minify to ^0.120.0
chore(deps): update dependency bun to v1.3.11
fix(deps): update dependency oxc-minify to ^0.121.0
…i, sqwish, crass)
BREAKING CHANGE: 5 deprecated compressor packages deleted. Removed all cross-references from cli config, core tests, examples, compressor-resolver, and compressor-registry.
- Replace @node-minify/run + google-closure-compiler-java with google-closure-compiler@20240317.0.0 - Use Compiler class JS API: options object + stdin pipe instead of raw process spawning - Add wrapMinificationError for consistent error handling - Repurpose types.d.ts for google-closure-compiler module declaration - Remove runCommandLine mocks from tests, add in-memory + invalid JS tests - All 25 tests pass, typecheck clean
isRemovedCompressor() needs these entries for CLI fail-fast and doctor command.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…e entry The astro v7 security bump (#2880) wrote a bun.lock referencing @napi-rs/wasm-runtime as a prod dependency of @astrojs/compiler-binding-wasm32-wasi without emitting the corresponding package entry, so `bun install --frozen-lockfile` failed to parse the lockfile and CI on develop went red. Regenerating also dedupes a stale nested @astrojs/cloudflare/astro tree.
Node 20 reached end-of-life on 2026-04-30 and was dropped from the CI matrix in 8360cab so the Astro v6 docs could build, leaving every package advertising engines.node >=20.0.0 that no CI run verified. Align the declared baseline with what is actually tested (22.x, 24.x): - Bump engines.node to >=22.0.0 across the root and all 22 published packages (packages/sharp was >=20.3.0). - Document the requirement in the v11 migration guide and getting started, and retarget the setup-node examples in the action docs. - Record the break in the v11 changeset. BREAKING CHANGE: Node.js 22 or later is now required.
🦋 Changeset detectedLatest commit: 1d8484c The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Important Review skippedToo many files! This PR contains 167 files, which is 17 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (167)
You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
13 issues found across 163 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/utils/__tests__/filesize-error-paths.test.ts">
<violation number="1" location="packages/utils/__tests__/filesize-error-paths.test.ts:29">
P3: Each test run leaks a unique `nm-filesize-*` directory (with `f.js`) into the OS temp dir because the `mkdtempSync` directory is never removed. Add an `afterAll` cleanup that removes `dir` recursively so runs don't accumulate temp artifacts.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:24">
P3: `unrun` is added to root devDependencies but is never referenced by any script, workflow, or config in the repo (the only match for "unrun" anywhere is the package.json entry itself). Either wire it into the build/CI scripts it was intended for, or drop it to avoid a dead dependency and the extra lockfile weight.</violation>
</file>
<file name="docs/src/content/docs/compressors/cssnano.md">
<violation number="1" location="docs/src/content/docs/compressors/cssnano.md:6">
P3: The new 'Status: Recommended' label on 8 compressor docs conflicts with Readme.md, which designates only terser and lightningcss as recommended (cssnano/esbuild/oxc/sharp/svgo/swc are listed without that label). Decide whether 'Recommended' means the top pick per category (matching the README) or the entire preferred set, and align both surfaces so the guidance isn't contradictory.</violation>
</file>
<file name="packages/benchmark/src/runner.ts">
<violation number="1" location="packages/benchmark/src/runner.ts:53">
P2: When a compressor returns multiple outputs, cleanup tracks only the base warmup path while core writes format-suffixed files, so warmups leave artifacts on disk. Track the resolved output paths for multi-output compressors before cleanup.</violation>
</file>
<file name="docs/package.json">
<violation number="1" location="docs/package.json:24">
P2: When this package installs with Astro 7, `@astrolib/analytics@0.6.1` has a peer range ending at Astro 5, so the docs project receives an incompatible peer dependency. Upgrade `@astrolib/analytics` to an Astro 7-compatible release or replace it with an explicitly compatible integration before adopting Astro 7.</violation>
</file>
<file name="docs/plans/2026-03-14-v11-plan.md">
<violation number="1" location="docs/plans/2026-03-14-v11-plan.md:31">
P3: The support-tier list only puts `jsonminify` under `Legacy`, but the shipped v11 registry `packages/utils/src/compressor-registry.ts` marks `no-compress` as `legacy` too (Legacy (2): jsonminify, no-compress), and `docs/plans/2026-06-22-v12-backlog.md` item 5 repeats the same pairing. This plan is merged as part of the v11 release, so its tier enumeration misleads readers about which compressors are legacy. Add `no-compress` to the Legacy tier.</violation>
<violation number="2" location="docs/plans/2026-03-14-v11-plan.md:71">
P3: The plan's `doctor` scope lists deprecated type-alias detection and Java-sensitive GCC/YUI guidance as v11 deliverables, but neither was shipped. `packages/cli/src/doctor.ts` only scans package.json, source imports, and workflow YAML for removed/legacy compressors (severity is `removed`/`legacy` only); `docs/plans/2026-06-22-v12-backlog.md` item 1 explicitly records that these two detections (plan lines 70-71) and the 'GCC runtime-guidance messaging' test were not implemented in v11 and deferred to v12. Merging this plan with the v11 release overstates what `node-minify doctor` does, so future readers will expect capabilities v11 does not have. Annotate these as deferred-to-v12 (or drop them) so the plan matches the shipped scope.</violation>
</file>
<file name=".github/actions/node-minify/action.yml">
<violation number="1" location=".github/actions/node-minify/action.yml:76">
P2: When a workflow passes `compressor: yui`, the install step still runs `bun add @node-minify/yui` (the yui entry remains in compressor-registry), but the yui Java setup and its deprecation warning were removed. yui requires Java, so remaining users get no explicit Java provisioning and no guidance, and fail only at minification time. Since yui is being dropped, fail fast instead: reject the `yui` input with a clear error in the alias mapping, or keep the Java setup and warning.</violation>
</file>
<file name="packages/google-closure-compiler/__tests__/google-closure-compiler.test.ts">
<violation number="1" location="packages/google-closure-compiler/__tests__/google-closure-compiler.test.ts:169">
P3: This unit test shells out to `bun run build` and depends on the `bun` executable being on PATH, so running the package's `vitest run` via npm (a Node-only environment) fails this file with ENOENT even though the compressor works. It also regenerates `dist/` in the working tree on every test run. Prefer importing the source under test (`../src/index.ts`, already imported for `applyOptions`) or gating the built-package smoke check behind a separate CI step, so a missing `bun` or a build failure can't fail the unit suite.</violation>
</file>
<file name="docs/plans/2026-06-22-v12-backlog.md">
<violation number="1" location="docs/plans/2026-06-22-v12-backlog.md:18">
P3: The claim that the compressor registry "already carries" the GCC runtime caveat via a `notes` field is inaccurate. `CompressorEntry` in `packages/utils/src/compressor-registry.ts` only has `name`, `status`, `packageName`, and optional `replacement`; there is no `notes` field and no Java/runtime-caveat text anywhere in the registry. A v12 implementer following this backlog would look for a field that does not exist and may assume the GCC guidance is already captured. Either add the `notes` field (with the GCC caveat) or reword this to say the note still needs to be added.</violation>
</file>
<file name="packages/cli/src/index.ts">
<violation number="1" location="packages/cli/src/index.ts:42">
P3: The fail-fast check runs before custom-compressor resolution, so it blocks any custom npm package whose name exactly matches a removed registry entry (e.g. a user's own package named 'crass' or 'yui') with a false 'removed in v11' error, even though the CLI documents custom npm compressor support. Gate the removed-compressor check to only fire when the entry actually corresponds to a built-in that can't resolve, or narrow the match so arbitrary custom packages aren't blocked.</violation>
</file>
<file name=".github/actions/node-minify/minify.ts">
<violation number="1" location=".github/actions/node-minify/minify.ts:112">
P3: yui still requires a type: @node-minify/yui@10.5.0 throws "You must specify a type: js or css" when `type` is absent, so removing yui from this guard does not relax a real requirement. It only moves the failure from the action's clear pre-flight error into a less specific "Minification failed" message thrown from inside the compressor. Keep yui in the requiresType check (and fix action.yml which claims type is required only for esbuild).</violation>
</file>
<file name="packages/action/__tests__/runAutoMode.test.ts">
<violation number="1" location="packages/action/__tests__/runAutoMode.test.ts:107">
P2: This test claims to verify outputDir normalization, but for outputDir "./" the ignore glob is "**/./**" whether or not the normalization code in src/index.ts runs, so the assertion passes even if the normalization regresses or is removed. Use outputDir "" as the input instead — the case the normalization fallback actually handles, where the un-normalized glob would be the invalid "**//**" — so the test provides real regression coverage for the behavior it names.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| // Track the path before minifying so a mid-loop failure still lets the | ||
| // caller delete the partial file instead of orphaning it. | ||
| collected.push(warmupOutput); |
There was a problem hiding this comment.
P2: When a compressor returns multiple outputs, cleanup tracks only the base warmup path while core writes format-suffixed files, so warmups leave artifacts on disk. Track the resolved output paths for multi-output compressors before cleanup.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/benchmark/src/runner.ts, line 53:
<comment>When a compressor returns multiple outputs, cleanup tracks only the base warmup path while core writes format-suffixed files, so warmups leave artifacts on disk. Track the resolved output paths for multi-output compressors before cleanup.</comment>
<file context>
@@ -32,23 +32,36 @@ import type {
+
+ // Track the path before minifying so a mid-loop failure still lets the
+ // caller delete the partial file instead of orphaning it.
+ collected.push(warmupOutput);
+
await minify({
</file context>
| "@astrolib/analytics": "0.6.1", | ||
| "@types/node": "^22.19.15", | ||
| "astro": "^5.18.0", | ||
| "astro": "^7.1.0", |
There was a problem hiding this comment.
P2: When this package installs with Astro 7, @astrolib/analytics@0.6.1 has a peer range ending at Astro 5, so the docs project receives an incompatible peer dependency. Upgrade @astrolib/analytics to an Astro 7-compatible release or replace it with an explicitly compatible integration before adopting Astro 7.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/package.json, line 24:
<comment>When this package installs with Astro 7, `@astrolib/analytics@0.6.1` has a peer range ending at Astro 5, so the docs project receives an incompatible peer dependency. Upgrade `@astrolib/analytics` to an Astro 7-compatible release or replace it with an explicitly compatible integration before adopting Astro 7.</comment>
<file context>
@@ -16,16 +16,16 @@
"@astrolib/analytics": "0.6.1",
"@types/node": "^22.19.15",
- "astro": "^5.18.0",
+ "astro": "^7.1.0",
"preact": "^10.28.4"
},
</file context>
| "rimraf": "6.1.3", | ||
| "tsdown": "^0.22.0", | ||
| "typescript": "5.9.3", | ||
| "unrun": "^0.3.1", |
There was a problem hiding this comment.
P3: unrun is added to root devDependencies but is never referenced by any script, workflow, or config in the repo (the only match for "unrun" anywhere is the package.json entry itself). Either wire it into the build/CI scripts it was intended for, or drop it to avoid a dead dependency and the extra lockfile weight.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 24:
<comment>`unrun` is added to root devDependencies but is never referenced by any script, workflow, or config in the repo (the only match for "unrun" anywhere is the package.json entry itself). Either wire it into the build/CI scripts it was intended for, or drop it to avoid a dead dependency and the extra lockfile weight.</comment>
<file context>
@@ -12,30 +12,30 @@
+ "rimraf": "6.1.3",
+ "tsdown": "^0.22.0",
"typescript": "5.9.3",
+ "unrun": "^0.3.1",
"vite-tsconfig-paths": "^6.1.1",
"vitest": "^4.0.18"
</file context>
| description: "cssnano for node-minify" | ||
| --- | ||
|
|
||
| **Status:** Recommended |
There was a problem hiding this comment.
P3: The new 'Status: Recommended' label on 8 compressor docs conflicts with Readme.md, which designates only terser and lightningcss as recommended (cssnano/esbuild/oxc/sharp/svgo/swc are listed without that label). Decide whether 'Recommended' means the top pick per category (matching the README) or the entire preferred set, and align both surfaces so the guidance isn't contradictory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/src/content/docs/compressors/cssnano.md, line 6:
<comment>The new 'Status: Recommended' label on 8 compressor docs conflicts with Readme.md, which designates only terser and lightningcss as recommended (cssnano/esbuild/oxc/sharp/svgo/swc are listed without that label). Decide whether 'Recommended' means the top pick per category (matching the README) or the entire preferred set, and align both surfaces so the guidance isn't contradictory.</comment>
<file context>
@@ -3,6 +3,8 @@ title: "cssnano"
description: "cssnano for node-minify"
---
+**Status:** Recommended
+
`cssnano` can compress only CSS files.
</file context>
The v11 `node-minify doctor` command shipped undocumented: it was referenced only in passing in the migration guide, with no usage, output or exit-code reference anywhere in the CLI docs or READMEs. - Add a Doctor Command section to the CLI docs covering usage, sample output and exit codes (1 on removed compressors, 0 on legacy-only) so it can be used as a CI gate. - Add a short Doctor section to the CLI package README. - Add Requirements (Node 22, ESM-only) and a v11 upgrade link to the root README, which previously stated neither.
Deploying node-minify-docs with
|
| Latest commit: |
ad0d150
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2237b0de.node-minify-docs.pages.dev |
| Branch Preview URL: | https://develop.node-minify-docs.pages.dev |
The doctor example in the CLI docs used a scoped @node-minify/babel-minify name, which the removed-compressor CI guard treats as a real usage reference (migration docs use bare names by convention), turning every test job red. Replace both lines with output captured verbatim from real doctor runs; the removed-compressor line now uses the bare workflow-YAML form. The previous sample was also inaccurate: doctor prints "Use terser instead.", not "Use @node-minify/terser instead."
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
doctor covered only compressor removals, so it was silent on 3 of v11's 5 breaking changes. A project depending on @node-minify/run, importing the removed type aliases and declaring engines.node >=18 produced no output and exited 0, reading as "v11-ready" when it was not. Add three detectors: - @node-minify/run in dependencies and source imports (error) - CompressorReturnType / MinifyOptions in TypeScript named imports, matched across multi-line blocks and resolving `as` renames (error) - engines.node ranges admitting a release below 22 (warning) Finding now carries a prebuilt message and an error/warning severity rather than a compressor-registry status, so non-compressor diagnostics share one reporting and exit-code path. Exempt doctor.ts from the guard's scoped scan and cli.md from its docs scan: as the removal detector and its documentation, both must name the packages they report on. Verified the guard still fails on a real violation elsewhere.
There was a problem hiding this comment.
1 issue found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/doctor.ts">
<violation number="1" location="packages/cli/src/doctor.ts:180">
P2: When `engines.node` uses an upper-bound or wildcard range, `checkNodeEngine` can report the project as clean even though the range allows Node below 22. Parse the range semantically, or conservatively warn for ranges that do not guarantee `>=22`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const nodeRange = (engines as Record<string, unknown>).node; | ||
| if (typeof nodeRange !== "string") return undefined; | ||
|
|
||
| const majors = [...nodeRange.matchAll(/(\d+)(?:\.\d+)*/g)] |
There was a problem hiding this comment.
P2: When engines.node uses an upper-bound or wildcard range, checkNodeEngine can report the project as clean even though the range allows Node below 22. Parse the range semantically, or conservatively warn for ranges that do not guarantee >=22.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/doctor.ts, line 180:
<comment>When `engines.node` uses an upper-bound or wildcard range, `checkNodeEngine` can report the project as clean even though the range allows Node below 22. Parse the range semantically, or conservatively warn for ranges that do not guarantee `>=22`.</comment>
<file context>
@@ -78,13 +109,89 @@ function buildEntryMap(
+ const nodeRange = (engines as Record<string, unknown>).node;
+ if (typeof nodeRange !== "string") return undefined;
+
+ const majors = [...nodeRange.matchAll(/(\d+)(?:\.\d+)*/g)]
+ .map((match) => Number(match[1]))
+ .filter((major) => Number.isFinite(major));
</file context>
@minify-html/node is a CommonJS native addon, so Node's ESM loader
exposes its exports only via the default export. The compressor read
`minify` off the namespace object, which is always undefined there, so
the published package threw "minifyHtmlLib.minify is not a function" on
every input when used from real Node.
CI never caught it: the error test mocked @minify-html/node with a flat
{ minify } shape that does not match the real module, and Vitest's CJS
interop resolved the namespace differently than Node. All 15 tests
passed against a package that could not run.
Also fix the docs this audit surfaced:
- options.md was entirely CommonJS require() with a removed `callback`
option, a removed `sync` option and a missing comma; rewritten as ESM
and awaited, with every example executed against the built packages.
- Add the missing minify-html and no-compress compressor pages and wire
them into the sidebar; minify-html is recommended tier yet had no page.
- List minify-html in the README HTML sections.
Codecov flagged doctor.ts line 154 (the newline counter in lineNumberAt) as the only uncovered line in the patch: every existing type-alias fixture put the import on line 1, where the loop body never runs. Add a fixture whose removed-alias import starts on line 4 and assert the reported location. Verified the test fails when the newline counter is removed.
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/src/content/docs/options.md">
<violation number="1" location="docs/src/content/docs/options.md:109">
P3: The new `buffer` section claims the option caps how much output a spawned compressor process may produce, but I found no code path that consumes `settings.buffer`: setup.ts only stores the default `1000 * 1024`, and core/utils never apply it. The example uses google-closure-compiler, whose implementation reads `settings.timeout` but not `settings.buffer`, so passing `buffer` there has no effect. Verify whether any compressor honors `buffer`; if not, reword the section (or drop the example) so the docs don't promise behavior the library doesn't implement.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| ## Max Buffer Size (only for Java) | ||
| ## Max Buffer Size | ||
|
|
||
| Some compressors spawn a child process. `buffer` caps how much output that process may produce before it is killed. The default is `1000 * 1024` bytes, which is enough for most files; raise it when minifying very large inputs. |
There was a problem hiding this comment.
P3: The new buffer section claims the option caps how much output a spawned compressor process may produce, but I found no code path that consumes settings.buffer: setup.ts only stores the default 1000 * 1024, and core/utils never apply it. The example uses google-closure-compiler, whose implementation reads settings.timeout but not settings.buffer, so passing buffer there has no effect. Verify whether any compressor honors buffer; if not, reword the section (or drop the example) so the docs don't promise behavior the library doesn't implement.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/src/content/docs/options.md, line 109:
<comment>The new `buffer` section claims the option caps how much output a spawned compressor process may produce, but I found no code path that consumes `settings.buffer`: setup.ts only stores the default `1000 * 1024`, and core/utils never apply it. The example uses google-closure-compiler, whose implementation reads `settings.timeout` but not `settings.buffer`, so passing `buffer` there has no effect. Verify whether any compressor honors `buffer`; if not, reword the section (or drop the example) so the docs don't promise behavior the library doesn't implement.</comment>
<file context>
@@ -123,21 +104,34 @@ When `allowEmptyOutput: true`:
-## Max Buffer Size (only for Java)
+## Max Buffer Size
+
+Some compressors spawn a child process. `buffer` caps how much output that process may produce before it is killed. The default is `1000 * 1024` bytes, which is enough for most files; raise it when minifying very large inputs.
+
+```js
</file context>
Suggested change
| Some compressors spawn a child process. `buffer` caps how much output that process may produce before it is killed. The default is `1000 * 1024` bytes, which is enough for most files; raise it when minifying very large inputs. | |
| Some compressors spawn a child process. If minifying very large inputs, you may need to raise the `buffer` size (default `1000 * 1024` bytes); verify that the chosen compressor actually honors it. |
Addresses cubic review findings on #2903, each reproduced first. npx (P1): every documented `npx node-minify doctor` resolves the unrelated unscoped node-minify@3.6.0, which ships its own node-minify bin, so the v11 scan never ran. Use `npx --package=@node-minify/cli -- node-minify doctor` in all five files, including the pre-existing occurrence in the changeset. Detection gaps (P2) — a project hitting all three exited 0: - side-effect `import "@node-minify/crass"` was invisible; IMPORT_REGEX now covers the bare import form. - `export type { MinifyOptions } from ...` was invisible; NAMED_IMPORT_REGEX now matches export declarations too. - `{ compressor : "yui" }` was invisible; COMPRESSOR_REGEX now allows whitespace before the colon. Verified each new test fails against the old regexes, and that valid v11 code still produces no findings.
Two further cubic findings on #2903, both reproduced first. A migration example written in a doc comment was matched as a real import, so doctor failed with an ERROR on correct code — the worst failure mode for a migration aid. Comments are now blanked before the type-alias scan, replacing each comment character with a space so byte offsets and reported line numbers are unchanged. engines.node ranges naming no version at all ("*", "x") produced no finding, because the major-version scan found nothing and returned early. Such a range places no floor on the runtime, so it is now reported rather than assumed safe. Verified a real import following commented examples is still flagged, with the correct line number.
There was a problem hiding this comment.
All reported issues were addressed across 7 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
More cubic findings on #2903, each verified. oxc-minify@0.148.0 requires `^20.19.0 || >=22.12.0`, but the package declared `>=22.0.0`, so on Node 22.0-22.11 it installs and then refuses to run. Since aligning the Node baseline is this release's point, set @node-minify/oxc to >=22.12.0. Checked every other runtime dependency; oxc is the only one with a floor above 22.0.0. The Windows CI legs run the removed-compressor guard through Git Bash. With no .gitattributes, Git for Windows checks out CRLF by default and the script dies on `set -euo pipefail\r` — reproduced locally as `set: pipefail: invalid option name`. Pin *.sh to LF. Docs: github-action.md recommended setup-node@v4 while the migration guide recommended v6 for the same Node 22 step; align on v6. AGENTS.md still claimed Node >=20.0.0.
There was a problem hiding this comment.
1 issue found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/doctor.ts">
<violation number="1" location="packages/cli/src/doctor.ts:468">
P2: When a source file contains `//` or `/*` inside a string, template, or regex literal before a real type import, `stripComments` blanks the literal text as a comment and the doctor misses the removed alias. Use a syntax-aware comment stripper that ignores comment markers inside literals.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| // migration example in a doc comment is not reported as a real | ||
| // import; blanking preserves offsets so line numbers stay correct. | ||
| if (TYPE_SOURCE_EXTENSIONS.has(extname(relPath))) { | ||
| const code = stripComments(content); |
There was a problem hiding this comment.
P2: When a source file contains // or /* inside a string, template, or regex literal before a real type import, stripComments blanks the literal text as a comment and the doctor misses the removed alias. Use a syntax-aware comment stripper that ignores comment markers inside literals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/doctor.ts, line 468:
<comment>When a source file contains `//` or `/*` inside a string, template, or regex literal before a real type import, `stripComments` blanks the literal text as a comment and the doctor misses the removed alias. Use a syntax-aware comment stripper that ignores comment markers inside literals.</comment>
<file context>
@@ -435,9 +461,12 @@ async function scanSourceImports(cwd: string): Promise<Finding[]> {
+ // import; blanking preserves offsets so line numbers stay correct.
if (TYPE_SOURCE_EXTENSIONS.has(extname(relPath))) {
- for (const match of content.matchAll(NAMED_IMPORT_REGEX)) {
+ const code = stripComments(content);
+ for (const match of code.matchAll(NAMED_IMPORT_REGEX)) {
const specifiers = match[1];
</file context>
The regex-based stripper had both failure directions. `/*` and `*/` appearing inside string literals formed a false comment spanning the code between them, hiding a real import (verified: a file with `const open = "/*"`, an import, then `const close = "*/"` reported nothing). A migration example embedded in a template literal was reported as a real import. Replace it with a single-pass scanner that walks quoted strings without altering them — an import specifier lives inside quotes and must still match — and blanks comment and template-literal bodies, preserving byte offsets so reported line numbers stay correct. Also scope the CLI docs note: only the type-alias check ignores comments, not every scanner.
The removed-compressor guard only scanned .ts/.js/.json/.md/.mdx/.yml/ .yaml, so an import of a removed package from .tsx, .jsx, .mjs, .cjs, .mts, .cts or an .astro file passed CI silently — verified for each extension. Add them to both the scoped and bare-name scans. README compressor comments predated the v11 support tiers and contradicted them: cssnano, esbuild, swc, oxc, sharp and svgo are 'recommended' in the registry but read as second-tier alternatives, while html-minifier and jsonminify carried no tier at all. Label every entry with its registry status.
The guard is a static grep over source, docs and action files with no OS or Node-version dependence, so running it on all four legs produced four identical results. Gate it to ubuntu/22.x, matching the existing runner.os condition on the Codecov step.
Codecov flagged doctor.ts:200-201 (the backslash branch in stripComments) as the only uncovered lines in the patch. The branch is load-bearing, not defensive: removing it and rebuilding makes doctor miss a real import entirely. Given `const quoted = "he said \\" /*";`, the escaped quote ends the string early, so the trailing /* opens a comment that swallows the import on the next line — verified as exit 0 with no findings. Add a fixture for that shape and assert the import is still reported. Verified the test fails when the branch is removed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2903 +/- ##
==========================================
+ Coverage 95.25% 99.64% +4.38%
==========================================
Files 73 69 -4
Lines 1770 1961 +191
Branches 552 599 +47
==========================================
+ Hits 1686 1954 +268
+ Misses 84 7 -77 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
v11 release: merge
developintomain149 commits. Merging this triggers the Changesets action on
main, which opens a "Version Packages" PR bumping all 22 published packages10.5.0 → 11.0.0. Publishing to npm happens when that PR is merged — not this one.Release-readiness fixes included
Two blockers were found while validating and are fixed in this branch:
1. Corrupt lockfile broke CI (
27074d53)The Astro v7 security bump (#2880) wrote a
bun.lockreferencing@napi-rs/wasm-runtimeas a prod dependency of@astrojs/compiler-binding-wasm32-wasiwithout emitting the corresponding package entry:bun install --frozen-lockfilefailed, so every job ondevelopwas red. Regenerating repaired the entry and deduped a stale nested@astrojs/cloudflare/astrotree (net -428 lines, all duplicate transitive deps — no top-level dependency lost).2. Untested EOL Node baseline (
3f77cc81) — breakingNode 20 was dropped from the CI matrix in 8360cab so Astro v6 docs could build, but all 22 packages still declared
engines.node >=20.0.0. v11 would have shipped a compatibility promise no CI run verified, for a runtime that hit EOL on 2026-04-30.Baseline raised to
>=22.0.0to match what is actually tested (22.x, 24.x): engines bumped across root + 22 packages (packages/sharpwas>=20.3.0), documented in the migration guide and getting-started,setup-nodeexamples in the action docs retargeted to 22, and the break recorded in the changeset.3.
node-minify doctorshipped undocumented (e5aadfeb,5de85b42)The flagship v11 CLI command was named once in passing in the migration guide, with no usage, sample output or exit-code reference anywhere in the CLI docs or READMEs. Added a Doctor Command section to the CLI docs (usage, real captured output, exit codes so it can gate CI), a short section to the CLI package README, and Requirements (Node 22, ESM-only) plus a v11 upgrade link to the root README, which stated neither.
Sample output is copied verbatim from real
doctorruns. The first draft was both inaccurate (doctorprintsUse terser instead., notUse @node-minify/terser instead.) and tripped the removed-compressor CI guard, which treats scoped@node-minify/<removed>names as real usage while migration docs use bare names.4.
doctormissed 3 of 5 breaking changes (168428b0)The command only understood compressor removals. A project depending on
@node-minify/run, importing the removedCompressorReturnType/MinifyOptionsaliases, and declaringengines.node >=18.0.0printed nothing and exited 0 — reading as "v11-ready" when it would fail outright. Added three detectors:@node-minify/runin deps and imports (error), removed type aliases in TypeScript named imports (error, handles multi-line blocks andasrenames), andengines.noderanges admitting Node below 22 (warning).Findingnow carries a prebuilt message plus an error/warning severity instead of a compressor-registry status, so non-compressor diagnostics share one reporting and exit-code path. 9 new tests, each verified to fail without the implementation.Verification
Full
bun run cilocally plus CI green on all 4 matrix legs (ubuntu/windows × Node 22/24) at27074d53:bun install --frozen-lockfilecheck-exportsAlso changed
Branch protection on
mainrequiredtest (*, 20.x)contexts the workflow can no longer produce — any PR would have blocked forever on missing checks. Required contexts now match the real matrix (22.x, 24.x).Notes, not blocking
postcss,sharp) are already resolved ondevelop— installedpostcss@8.5.26(patched 8.5.18),sharp@0.35.4(patched 0.35.0). They are stale againstmainand this merge clears them.@astrojs/cloudflare@13.7.0peer-warns againstastro@7.2.10(wants^6.3.0); adapter v14 supports Astro 7. Docs-only,private: true, not published, and the docs build passes. Worth a separate Renovate bump.packages/action/action.ymlstill declaresusing: "node20". That is the GitHub Action runner runtime on the independentsrod/node-minify@v1line, not the npm packages; changing it forces an action major bump, so it is deliberately out of scope here.