Add CLI version #384
Add CLI version #384
Conversation
It's needed for the CLI later.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds a new CLI entry point and related tests, updates package.json to expose the binary and add a dependency, exports presets, shifts configPath into HtmlnanoOptions, updates types and tests accordingly, and expands documentation (including a duplicated CLI section in docs/020-usage.md). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant C as CLI (Commander)
participant H as htmlnano.process
participant L as loadConfig
participant FS as File System
U->>C: Run htmlnano [--preset/-p] [--config/-c] [-i/-o]
C->>C: Validate preset against exported presets
alt invalid preset
C-->>U: Print error with available presets
C-->C: set process.exitCode = 1
else valid preset
opt read input
alt -i provided
C->>FS: Read input file
FS-->>C: HTML
else stdin
U-->>C: HTML via STDIN
end
end
C->>H: processHtml(html, {configPath, ...}, selectedPreset)
H->>L: loadConfig(options.configPath, options)
L-->>H: Resolved options
H-->>C: Minified HTML
opt write output
alt -o provided
C->>FS: Write output file
FS-->>C: Done
else stdout
C-->>U: Minified HTML via STDOUT
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bin/index.ts (1)
15-45: Handle async action errors viaparseAsync()(or explicit try/catch).
program.parse()does not await or catch the promise returned by the async action. Any rejection fromprocessHtml(e.g. invalid config) will surface as an unhandled promise rejection and skip Commander’s error handling. Switch toprogram.parseAsync(process.argv)or wrap the action body in try/catch and setprocess.exitCode.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
README.md(1 hunks)docs/docs/020-usage.md(1 hunks)docs/docs/030-config.md(1 hunks)package.json(3 hunks)src/bin/index.ts(1 hunks)src/index.ts(1 hunks)src/types.ts(2 hunks)test/htmlnano.ts(2 hunks)test/usage_cli.ts(1 hunks)
🔇 Additional comments (8)
package.json (1)
60-60: Verify Node.js support before adopting commander v14.Commander 14 requires Node.js 20+. htmlnano previously supported older LTS versions, so this dependency effectively raises the minimum Node version. Confirm that the project is ready to drop support for older runtimes or stick to commander 13.x.
Based on learnings
src/index.ts (2)
11-15: LGTM!Exporting the
presetsconstant provides a clean public API for the CLI and external consumers to discover available presets.
17-21: AllloadConfigcalls now use the new signature; no instances of a thirdconfigPathargument remain. This is a breaking change—add a note to the CHANGELOG.test/usage_cli.ts (5)
14-20: LGTM!The test correctly validates stdin/stdout processing by passing input via
execFileSyncand asserting the minified output.
22-28: LGTM!The test correctly validates the
--preset maxoption by asserting the more aggressively minified output.
30-40: LGTM!The test correctly validates error handling for an invalid preset by asserting non-zero exit status, empty stdout, and an informative error message listing available presets.
42-61: LGTM!The test correctly validates config file loading via the
-cflag. The temporary file is properly cleaned up in thefinallyblock, ensuring no test artifacts remain.
63-84: LGTM!The test correctly validates file-based I/O by reading from an input file and writing to an output file. The assertions comprehensively cover exit status, stdout emptiness, and output file content. Cleanup is properly handled for both temporary files.
| "scripts": { | ||
| "clean": "rimraf dist", | ||
| "build": "npm run clean && bunchee", | ||
| "postbuild": "chmod +x dist/bin.js", |
There was a problem hiding this comment.
Avoid platform-specific chmod in npm scripts.
Running npm run build on Windows will fail because chmod is not available in the default shell, breaking local builds and CI on that platform. Please switch to a cross-platform approach, e.g. invoking node -e "require('fs').chmodSync('dist/bin.js', 0o755)" or configuring Bunchee to preserve the executable bit.
🤖 Prompt for AI Agents
In package.json around line 10, the "postbuild" script uses a platform-specific
chmod command which fails on Windows; replace it with a cross-platform
Node-based chmod (or configure Bunchee to preserve executable bit) by invoking a
small node.js one-liner that calls fs.chmodSync on dist/bin.js so the executable
bit is set on all platforms.
For now, I made it quite simple: the options can be passed only via the configuration file.
Later, we might add CLI args for the options.
The help section:
It relies on #383 bug fix.
Original issue: #3
Summary by CodeRabbit
New Features
Documentation
Tests