-
Notifications
You must be signed in to change notification settings - Fork 50
feat(cli,benchmark): add support for custom compressors #2736
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
b8f0241
docs: add custom compressors guide
srod 29e307f
feat(cli,benchmark): add support for custom compressors
srod 799fcc4
test(utils): improve compressor-resolver coverage
srod 619ce11
fix(test): handle Windows path separators in relative path test
srod 9081e03
fix: address code review feedback
srod e33531d
fix(utils): add compressor result validation and address reviews
srod 134ba26
test: add retry logic for fs.cpSync in fixtures to reduce Windows CI …
srod ce47884
docs: correct export resolution order in custom-compressors.md
srod 62e913c
docs: clarify compressor registration steps
srod 41c5ee0
test: clear module cache in compressor-resolver tests
srod f87f112
test: use error.code instead of message for fs retry logic
srod 2f84484
test: use Atomics.wait for blocking sleep instead of busy-wait
srod 64aeec1
docs: explain why two registration steps are needed
srod d0dab12
style: fix non-null assertion lint warning
srod 565662c
📝 Add docstrings to `docs/custom-compressors`
coderabbitai[bot] 7184d5e
Merge pull request #2737 from srod/coderabbitai/docstrings/d0dab12
srod a7171d8
test(utils): add coverage for local file error handling branches
srod 1eea831
test(utils): improve compressor-resolver test coverage to 100%
srod 1c27a57
style: add missing newlines at end of files
srod File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| # Plan: Enable CLI & Benchmark Support for Custom Compressors | ||
|
|
||
| ## Objective | ||
| Allow developers to use custom compressors (external packages or local files) with the `node-minify` CLI and Benchmark tools, without requiring them to be hardcoded in the core repository's registry files. | ||
|
|
||
| --- | ||
|
|
||
| ## Status: IMPLEMENTED ✅ | ||
|
|
||
| Completed January 2026. The CLI and Benchmark tools now support custom compressors via: | ||
| - Built-in `@node-minify/*` packages (existing behavior) | ||
| - External npm packages (`node-minify --compressor my-custom-compressor`) | ||
| - Local file paths (`node-minify --compressor ./my-compressor.js`) | ||
|
|
||
| Shared resolver utility added to `@node-minify/utils` (`compressor-resolver.ts`). | ||
|
|
||
| --- | ||
|
|
||
| ## Problem | ||
| Currently, the CLI and Benchmark tools rely on static arrays/objects (`AVAILABLE_MINIFIER` in `packages/cli/src/config.ts` and `COMPRESSOR_EXPORTS` in `packages/benchmark/src/compressor-loader.ts`) to map a string name (e.g., "terser") to a package import. This prevents users from using their own compressors via the command line, e.g., `node-minify --compressor my-custom-compressor`. | ||
|
|
||
| ## Proposed Solution | ||
| Modify the CLI and Benchmark tools to support dynamic resolution of compressor names. | ||
|
|
||
| ### 1. CLI Logic Update (`packages/cli/src/index.ts`) | ||
| Current logic: | ||
| 1. Look up name in `AVAILABLE_MINIFIER`. | ||
| 2. If found, dynamic import `@node-minify/<name>`. | ||
|
|
||
| New logic: | ||
| 1. Look up name in `AVAILABLE_MINIFIER`. | ||
| 2. If found, use the standard logic (import `@node-minify/<name>`). | ||
| 3. **If NOT found**: | ||
| * Attempt to dynamic import the name directly (e.g., `import("my-custom-pkg")`). | ||
| * If that fails, attempt to resolve it relative to the current working directory (for local files). | ||
| * Expect the module to export a compressor function (default export or matching name). | ||
|
|
||
| ### 2. Benchmark Logic Update (`packages/benchmark/src/compressor-loader.ts`) | ||
| Current logic: | ||
| 1. Look up name in `COMPRESSOR_EXPORTS`. | ||
| 2. Import `@node-minify/<name>`. | ||
|
|
||
| New logic: | ||
| 1. Similar fallback strategy as the CLI. | ||
| 2. If the known export map fails, try importing the package/file directly. | ||
|
|
||
| ## Implementation Details | ||
|
|
||
| ### Step 1: Update CLI (`packages/cli`) | ||
| * **File**: `packages/cli/src/index.ts` (function `runOne`, lines 38-94) | ||
| * **Change**: Refactor the "Find compressor" block (lines 40-46). | ||
| * If `minifierDefinition` is missing, do NOT throw immediately. | ||
| * Instead, try to load the module using the provided string. | ||
| * Check for exports in this priority order: | ||
| 1. Named export matching camelCase of package name (e.g., `myTool` from `my-tool`) | ||
| 2. Named export `compressor` | ||
| 3. Default export | ||
| 4. First function export | ||
|
|
||
| ### Step 2: Update Benchmark (`packages/benchmark`) | ||
| * **File**: `packages/benchmark/src/compressor-loader.ts` (function `loadCompressor`, lines 33-45) | ||
| * **Change**: Refactor to support arbitrary package names. | ||
| * If `COMPRESSOR_EXPORTS[name]` is undefined, assume the package name is the CLI argument. | ||
| * Try `import(name)`. | ||
| * Fallback to `mod.default`. | ||
|
|
||
| ### Step 3: Update Documentation | ||
| * Update `docs/src/content/docs/custom-compressors.md` to explain how to use this new feature. | ||
| * Example: `node-minify --compressor my-published-package ...` | ||
| * Example: `node-minify --compressor ./local-compressor.js ...` | ||
|
|
||
| ### Step 4: Update CLI Help Text | ||
| * Update `--compressor` option description: | ||
| ``` | ||
| --compressor <name> Built-in compressor name, npm package, or path to local file | ||
| ``` | ||
|
|
||
| ## Risks & Considerations | ||
| * **Security**: dynamically importing arbitrary strings from CLI arguments can be risky if inputs aren't sanitized, but in a CLI tool context, the user is already executing code they control. We should ensure we handle import errors gracefully. | ||
| * **ESM vs CJS**: We are using `import()`, which should handle both in modern Node/Bun environments, but we need to ensure local file paths are absolute or properly formatted for `import()` (e.g., `file://...`). | ||
|
|
||
| --- | ||
|
|
||
| ## Additional Considerations (Added During Review) | ||
|
|
||
| ### TypeScript Type Changes | ||
| The current type in `packages/cli/src/index.ts` (line 16-18): | ||
| ```typescript | ||
| export type SettingsWithCompressor = Omit<Settings, "compressor"> & { | ||
| compressor: (typeof AVAILABLE_MINIFIER)[number]["name"]; | ||
| }; | ||
| ``` | ||
| After allowing arbitrary strings, this needs to become: | ||
| ```typescript | ||
| export type SettingsWithCompressor = Omit<Settings, "compressor"> & { | ||
| compressor: string; | ||
| }; | ||
| ``` | ||
|
|
||
| ### File URL Handling for Local Paths | ||
| For local file resolution, use proper `file://` URL format: | ||
| ```typescript | ||
| import path from "node:path"; | ||
|
|
||
| async function resolveCompressor(name: string) { | ||
| // Try known compressors first | ||
| const known = AVAILABLE_MINIFIER.find(c => c.name === name); | ||
| if (known) { | ||
| return import(`@node-minify/${name}`); | ||
| } | ||
|
|
||
| // Try as npm package | ||
| try { | ||
| return await import(name); | ||
| } catch { | ||
| // Try as local file | ||
| const isLocalPath = name.startsWith('./') || name.startsWith('/') || name.startsWith('../'); | ||
| if (isLocalPath) { | ||
| const absolutePath = path.resolve(process.cwd(), name); | ||
| const fileUrl = new URL(`file://${absolutePath}`).href; | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
|
||
| return await import(fileUrl); | ||
| } | ||
| throw new Error(`Could not resolve compressor '${name}'. Is it installed?`); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Error Messages | ||
| Add specific error messages for different failure modes: | ||
| - Package not found: `"Could not resolve compressor '{name}'. Is it installed?"` | ||
| - No valid export: `"Package '{name}' doesn't export a valid compressor function. Expected a function as default export or named export 'compressor'."` | ||
| - Invalid return type: `"Compressor '{name}' returned invalid result. Expected { code: string }."` | ||
|
|
||
| ### Compressor Return Validation | ||
| Add runtime validation that the dynamically loaded function returns a proper `CompressorResult`: | ||
| ```typescript | ||
| function validateCompressorResult(result: unknown, name: string): asserts result is CompressorResult { | ||
| if (!result || typeof result !== 'object') { | ||
| throw new Error(`Compressor '${name}' returned invalid result`); | ||
| } | ||
| if (!('code' in result) || typeof (result as any).code !== 'string') { | ||
| throw new Error(`Compressor '${name}' must return { code: string }`); | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| ### Registry Deduplication | ||
| Consider extracting shared resolution logic to avoid duplication between: | ||
| - `packages/cli/src/config.ts` (`AVAILABLE_MINIFIER`) | ||
| - `packages/benchmark/src/compressor-loader.ts` (`COMPRESSOR_EXPORTS`) | ||
|
|
||
| Possible shared utility in `@node-minify/utils`: | ||
| ```typescript | ||
| // packages/utils/src/compressor-resolver.ts | ||
| export async function resolveCompressor(name: string): Promise<Compressor> | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Test Cases | ||
|
|
||
| ### Required Tests | ||
| 1. **Built-in compressor**: `node-minify --compressor terser` (existing behavior) | ||
| 2. **Published npm package**: `node-minify --compressor my-custom-compressor` | ||
| 3. **Local relative path**: `node-minify --compressor ./compressor.js` | ||
| 4. **Local absolute path**: `node-minify --compressor /path/to/compressor.js` | ||
| 5. **Invalid package**: Should fail gracefully with helpful error | ||
| 6. **Package without valid export**: Should fail with clear message about expected exports | ||
| 7. **Compressor returns invalid result**: Should fail with validation error | ||
|
|
||
|
srod marked this conversation as resolved.
|
||
| ### Test File Template | ||
| ```javascript | ||
| // test-compressor.js | ||
| export default async function({ content }) { | ||
| return { code: content.replace(/\s+/g, ' ') }; | ||
| } | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Verification | ||
| * Create a temporary local compressor file. | ||
| * Run the CLI pointing to it. | ||
| * Verify it executes. | ||
| * Run benchmark with custom compressor. | ||
| * Verify error messages for failure cases. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,101 @@ | ||
| # Plan: Custom & Contributed Compressor Guidelines | ||
|
|
||
| ## Objective | ||
| Enable developers to create their own compressors for `node-minify`. This covers two use cases: | ||
| 1. **External Plugins**: Developers creating standalone packages or local functions for their own projects. | ||
| 2. **Official Contributions**: Developers adding a new compressor to the `node-minify` monorepo. | ||
|
|
||
| --- | ||
|
|
||
| ## Status: IMPLEMENTED ✅ | ||
|
|
||
| Completed January 2026. | ||
|
|
||
| ### Completed | ||
| - [x] Documentation created at `docs/src/content/docs/custom-compressors.md` | ||
| - [x] Sidebar updated in `docs/src/consts.ts` (line 12) | ||
| - [x] Compressor interface documented with TypeScript types | ||
| - [x] Minimal example with inline compressor | ||
| - [x] Handling options example | ||
| - [x] Contributing to Core section (package structure, CLI registration, Benchmark registration, testing) | ||
| - [x] CLI and Benchmark support for custom compressors (npm packages and local files) | ||
|
|
||
| ### Remaining Items | ||
| - [ ] **Binary content support**: Add note that compressors can handle `Buffer` content (for image compressors like `sharp`, `imagemin`) | ||
| - [ ] **Source map support**: Document that compressors can optionally return `map` field | ||
| - [ ] **CONTRIBUTING.md**: Consider adding/linking compressor contribution guide in root `CONTRIBUTING.md` | ||
|
|
||
| --- | ||
|
|
||
| ## 1. External Plugins (Library Mode) | ||
| *Context: You are a user who wants to use a custom tool with `node-minify`.* | ||
|
|
||
| **Concept:** | ||
| The `minify()` function accepts a `Compressor` function. You do **not** need to register your compressor in any static lists (`AVAILABLE_MINIFIER`, etc.). Those lists are only for built-in compressors. | ||
|
|
||
| **Action Items:** | ||
| * ~~Create `docs/src/content/docs/custom-compressors.md`.~~ DONE | ||
| * **Guide Content**: | ||
| * ~~**The Compressor Interface**: Explain `MinifierOptions` and `CompressorResult`.~~ DONE | ||
| * ~~**Example**: Writing a simple in-memory replacement compressor.~~ DONE | ||
| * ~~**Usage**: Passing the function directly to `minify({ compressor: myFunc })`.~~ DONE | ||
| * ~~**CLI Support**: Document npm package and local file path usage.~~ DONE | ||
|
srod marked this conversation as resolved.
|
||
|
|
||
| ## 2. Official Contributions (Core Integration) | ||
| *Context: You are contributing a new compressor package (e.g., `@node-minify/new-tool`) to the repository.* | ||
|
|
||
| **Concept:** | ||
| To expose a new compressor via the CLI and Benchmark tools, it must be registered in the static configuration files. | ||
|
|
||
| **Action Items:** | ||
| * ~~Add a "Contributing a Compressor" section to the docs.~~ DONE (in custom-compressors.md) | ||
| * Consider also adding to a separate internal guide `CONTRIBUTING.md` **TODO** | ||
| * **Checklist for New Compressors**: (All documented in custom-compressors.md) | ||
| 1. ~~**Package Creation**: Create `packages/<name>` with standard structure.~~ DONE | ||
| 2. ~~**CLI Registration**: Update `packages/cli/src/config.ts`: Add to `AVAILABLE_MINIFIER` array.~~ DONE | ||
| 3. ~~**Benchmark Registration**: Update `packages/benchmark/src/compressor-loader.ts`: Add to `COMPRESSOR_EXPORTS`.~~ DONE | ||
| 4. ~~**Tests**: Add tests using shared helpers.~~ DONE | ||
|
|
||
| ## Implementation Steps | ||
|
|
||
| ### Documentation | ||
| 1. ~~**Create** `docs/src/content/docs/custom-compressors.md`~~ DONE | ||
| 2. ~~**Update** `docs/src/consts.ts`: Add "Custom Compressors" to the sidebar.~~ DONE | ||
|
|
||
| ## Verification | ||
| * ~~Verify that `AVAILABLE_MINIFIER` and `COMPRESSOR_EXPORTS` are the only two registry files.~~ Confirmed: Yes | ||
| * Note: `tests/fixtures.ts` does not have a static list of compressors - tests are defined per-package | ||
|
|
||
| --- | ||
|
|
||
| ## Suggested Additions to Existing Docs | ||
|
|
||
| ### 1. Add Binary Content Note | ||
| ```markdown | ||
| ### Binary Content (Images) | ||
|
|
||
| For image compressors, the `content` parameter may be a `Buffer` instead of a string. | ||
| Your compressor should return `buffer` in the result for binary output: | ||
|
|
||
| \`\`\`typescript | ||
| export const myImageCompressor: Compressor = async ({ content }) => { | ||
| const buffer = content as Buffer; | ||
| const optimized = await processImage(buffer); | ||
| return { code: "", buffer: optimized }; | ||
| }; | ||
| \`\`\` | ||
| ``` | ||
|
|
||
| ### 2. Add Source Map Note | ||
| ```markdown | ||
| ### Source Maps | ||
|
|
||
| Compressors can optionally return source maps: | ||
|
|
||
| \`\`\`typescript | ||
| return { | ||
| code: minifiedCode, | ||
| map: sourceMapString // Optional | ||
| }; | ||
| \`\`\` | ||
| ``` | ||
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.