fix: 优化 lint 执行策略并修复 pre-commit 依赖解析 - #8071
Conversation
- vsh lint 按硬件自适应:CPU<=4 核串行(汇总错误),>4 核并行; CPU>4 且空闲内存>8GB 时 oxfmt/oxlint 默认线程数提升至 4,否则维持 2 - lefthook pre-commit 改为串行执行,避免与 check:type 并行导致低配机资源飙升 - pre-commit 仅对暂存文件 lint 并自动修复回填(stage_fixed) - lint 命令统一加 pnpm exec 前缀,修复 lefthook 经 sh 调用时 oxlint 找不到 workspace 局部依赖(eslint-plugin-eslint-comments)的问题
|
📝 WalkthroughWalkthroughThe lint script ( ChangesResource-aware lint execution and pre-commit pipeline
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
scripts/vsh/src/lint/index.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lefthook.yml`:
- Line 50: The oxlint command in the pre-commit hook configuration has a
hardcoded --threads=4 parameter that may cause resource exhaustion on
constrained machines. Replace the hardcoded --threads=4 with a safer default
value such as --threads=2, or remove the threads parameter entirely to allow
oxlint to use its default adaptive behavior. Apply this same change to the
second occurrence of the oxlint command that also contains the hardcoded thread
count.
In `@scripts/vsh/src/lint/index.ts`:
- Line 92: The `--threads` option is being parsed as a string by the `cac`
library, but `LintCommandOptions` expects a number, and the falsy check on line
92 incorrectly treats string "0" as falsy. Update the option definition where
`--threads` is configured to include type coercion (convert the parsed string to
a number), then add validation logic to ensure the parsed value is a valid
positive number or zero before constructing the `threadsArg`. This ensures
invalid values are caught with clear errors rather than being passed to the
underlying tools.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c54baa41-3c83-4f2d-8aac-19dff46aa992
📒 Files selected for processing (2)
lefthook.ymlscripts/vsh/src/lint/index.ts
| jobs: | ||
| - name: oxlint | ||
| glob: '*.{js,jsx,ts,tsx,vue,cjs,mjs,cts,mts}' | ||
| run: pnpm exec oxlint --fix --type-aware --threads=4 --no-error-on-unmatched-pattern {staged_files} |
There was a problem hiding this comment.
Hardcoded --threads=4 in pre-commit can reintroduce low-spec resource failures.
This bypasses the adaptive thread strategy and may still overload constrained machines. Prefer a safer default (e.g., 2) or make thread count configurable.
Proposed fix
- run: pnpm exec oxlint --fix --type-aware --threads=4 --no-error-on-unmatched-pattern {staged_files}
+ run: pnpm exec oxlint --fix --type-aware --threads=2 --no-error-on-unmatched-pattern {staged_files}
@@
- run: pnpm exec oxfmt --threads=4 --no-error-on-unmatched-pattern {staged_files}
+ run: pnpm exec oxfmt --threads=2 --no-error-on-unmatched-pattern {staged_files}Also applies to: 54-54
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lefthook.yml` at line 50, The oxlint command in the pre-commit hook
configuration has a hardcoded --threads=4 parameter that may cause resource
exhaustion on constrained machines. Replace the hardcoded --threads=4 with a
safer default value such as --threads=2, or remove the threads parameter
entirely to allow oxlint to use its default adaptive behavior. Apply this same
change to the second occurrence of the oxlint command that also contains the
hardcoded thread count.
| // 用户通过 --threads 显式指定时优先使用其值。 | ||
| const defaultThreads = | ||
| cpuCores > CPU_CORE_THRESHOLD && freemem() > FREE_MEMORY_THRESHOLD ? 4 : 2; | ||
| const threadsArg = ` --threads=${threads || defaultThreads}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n scripts/vsh/src/lint/index.ts | head -120 | tail -40Repository: vbenjs/vue-vben-admin
Length of output: 1455
🏁 Script executed:
rg -A 20 "LintCommandOptions" scripts/vsh/src/lint/index.tsRepository: vbenjs/vue-vben-admin
Length of output: 1179
🏁 Script executed:
rg -B 10 "const threadsArg" scripts/vsh/src/lint/index.tsRepository: vbenjs/vue-vben-admin
Length of output: 458
🏁 Script executed:
rg "threads" scripts/vsh/src/lint/index.ts -A 2 -B 2Repository: vbenjs/vue-vben-admin
Length of output: 1178
🏁 Script executed:
rg -B 30 "\.option\('--threads" scripts/vsh/src/lint/index.ts | head -50Repository: vbenjs/vue-vben-admin
Length of output: 874
🏁 Script executed:
rg "\.action\(" scripts/vsh/src/lint/index.ts -A 5 -B 5Repository: vbenjs/vue-vben-admin
Length of output: 319
🏁 Script executed:
rg "import.*cac" scripts/vsh/src/lint/index.tsRepository: vbenjs/vue-vben-admin
Length of output: 97
🏁 Script executed:
fd "index.ts" scripts/vsh/src/lint -type f -exec wc -l {} \;Repository: vbenjs/vue-vben-admin
Length of output: 236
🏁 Script executed:
cat -n scripts/vsh/src/lint/index.ts | tail -30Repository: vbenjs/vue-vben-admin
Length of output: 1008
🏁 Script executed:
rg "cac\|CAC" scripts/vsh/src/lint/index.ts -B 5 -A 15Repository: vbenjs/vue-vben-admin
Length of output: 47
🏁 Script executed:
fd "cac" scripts/vsh/src --type f | head -5Repository: vbenjs/vue-vben-admin
Length of output: 47
🏁 Script executed:
rg "cac.*parse\|\.parse\(" scripts/vsh/src -A 5 -B 5 | head -40Repository: vbenjs/vue-vben-admin
Length of output: 47
🏁 Script executed:
find scripts/vsh/src -name "*.ts" -type f | head -20Repository: vbenjs/vue-vben-admin
Length of output: 268
🏁 Script executed:
rg "defineLintCommand" scripts/vsh/src -B 5 -A 5Repository: vbenjs/vue-vben-admin
Length of output: 2142
🏁 Script executed:
rg "\.parse\(" scripts/vsh/src/bin -A 10 -B 5Repository: vbenjs/vue-vben-admin
Length of output: 125
🏁 Script executed:
cat -n scripts/vsh/src/index.ts | tail -20Repository: vbenjs/vue-vben-admin
Length of output: 619
🏁 Script executed:
rg "\.parse\(" scripts/vsh/src/index.ts -A 5 -B 5Repository: vbenjs/vue-vben-admin
Length of output: 478
🏁 Script executed:
web_search
cac CLI library default behavior threads option parsing type coercionRepository: vbenjs/vue-vben-admin
Length of output: 158
🌐 Web query:
cac library JavaScript --option <value> default type string coercion
💡 Result:
In the cac library, you can manage option types and coercion through the configuration object provided as the third argument to the cli.option method [1][2][3]. To define the type and coercion for an option, use the type property within the OptionConfig object [4]. This property accepts an array [1][3]. 1. Basic Type Transformation: You can pass a constructor or conversion function (such as [String], [Number], or custom functions like [JSON.parse]) inside the array [4][5]. CAC will automatically invoke this function on the provided option value [1][4]. Example: cli.option('--timeout ', 'Timeout in milliseconds', { type: [Number], default: 5000 }); cli.option('--data ', 'JSON input', { type: [JSON.parse] }); 2. Array Types: If you set the type property to an empty array [], the option value will be treated as an array [1][4][3]. 3. Default Values: You can specify a default value using the default property in the same configuration object [1][4]. Note on Coercion: While some documentation and recent development discussions highlight ongoing efforts to standardize default string conversion [6], the primary mechanism for explicit coercion in the current stable API is the type property using the array-based transformation function syntax [1][4]. Ensure your implementation uses the [Function] syntax (e.g., [String]) to correctly register the transformation [1][3].
Citations:
- 1: https://www.npmjs.com/package/cac
- 2: https://github.com/cacjs/cac/blob/master/README.md
- 3: https://npmx.dev/package/cac/v/7.0.0
- 4: https://tessl.io/registry/tessl/npm-cac/6.0.0/files/docs/option-system.md
- 5: https://cdn.jsdelivr.net/npm/cac@6.1.3/README.md
- 6: feat: Option type default to string cacjs/cac#163
🏁 Script executed:
rg "Number\|parseInt\|coerce" scripts/vsh/src/lint/index.tsRepository: vbenjs/vue-vben-admin
Length of output: 47
🏁 Script executed:
cat -n scripts/vsh/src/lint/index.ts | head -100Repository: vbenjs/vue-vben-admin
Length of output: 3114
Add type coercion and validation for --threads option.
The cac library parses --threads as a string by default, but LintCommandOptions expects a number. The code lacks both type coercion and validation, allowing invalid values to reach oxfmt/oxlint with opaque tool errors. Additionally, string "0" would pass the falsy check and be used instead of the default. Configure type coercion in the option definition and validate the parsed value.
Proposed fix
function defineLintCommand(cac: CAC) {
cac
.command('lint')
.usage('Batch execute project lint check.')
.option('--format', 'Format lint problem.')
- .option('--threads <count>', 'Number of threads for oxfmt and oxlint.')
+ .option('--threads <count>', 'Number of threads for oxfmt and oxlint.', { type: [Number] })
.action(runLint);
}
async function runLint({ format, threads }: LintCommandOptions) {
const cpuCores = availableParallelism();
const defaultThreads =
cpuCores > CPU_CORE_THRESHOLD && freemem() > FREE_MEMORY_THRESHOLD ? 4 : 2;
- const threadsArg = ` --threads=${threads || defaultThreads}`;
+ if (threads != null && (!Number.isInteger(threads) || threads <= 0)) {
+ throw new Error(`Invalid --threads value: ${threads}`);
+ }
+ const threadsArg = ` --threads=${threads ?? defaultThreads}`;📝 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.
| const threadsArg = ` --threads=${threads || defaultThreads}`; | |
| function defineLintCommand(cac: CAC) { | |
| cac | |
| .command('lint') | |
| .usage('Batch execute project lint check.') | |
| .option('--format', 'Format lint problem.') | |
| .option('--threads <count>', 'Number of threads for oxfmt and oxlint.', { type: [Number] }) | |
| .action(runLint); | |
| } | |
| async function runLint({ format, threads }: LintCommandOptions) { | |
| const cpuCores = availableParallelism(); | |
| const defaultThreads = | |
| cpuCores > CPU_CORE_THRESHOLD && freemem() > FREE_MEMORY_THRESHOLD ? 4 : 2; | |
| if (threads != null && (!Number.isInteger(threads) || threads <= 0)) { | |
| throw new Error(`Invalid --threads value: ${threads}`); | |
| } | |
| const threadsArg = ` --threads=${threads ?? defaultThreads}`; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/vsh/src/lint/index.ts` at line 92, The `--threads` option is being
parsed as a string by the `cac` library, but `LintCommandOptions` expects a
number, and the falsy check on line 92 incorrectly treats string "0" as falsy.
Update the option definition where `--threads` is configured to include type
coercion (convert the parsed string to a number), then add validation logic to
ensure the parsed value is a valid positive number or zero before constructing
the `threadsArg`. This ensures invalid values are caught with clear errors
rather than being passed to the underlying tools.
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
Release Notes