Skip to content

fix: 优化 lint 执行策略并修复 pre-commit 依赖解析 - #8071

Merged
jinmao88 merged 1 commit into
vbenjs:mainfrom
xingyu4j:lint-config
Jun 21, 2026
Merged

jinmao88 merged 1 commit into
vbenjs:mainfrom
xingyu4j:lint-config

Conversation

@xingyu4j

@xingyu4j xingyu4j commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
  • 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)的问题

Description

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Checklist

ℹ️ Check all checkboxes - this will indicate that you have done everything in accordance with the rules in CONTRIBUTING.

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs:dev command.
  • Run the tests with pnpm test.
  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

Release Notes

  • Chores
    • Reorganized pre-commit hook pipeline to run linting fixers sequentially with dedicated processes for oxlint, oxfmt, eslint, and stylelint, followed by type checking.
    • Enhanced lint execution to dynamically adapt concurrency strategy based on available system resources, automatically optimizing performance across different machine configurations.

- 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)的问题
@changeset-bot

changeset-bot Bot commented Jun 18, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: dc3865f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The lint script (scripts/vsh/src/lint/index.ts) gains resource-based execution logic: OS CPU and memory metrics drive a choice between new runSerial and runParallel helpers, and runLint now computes thread count and execution mode dynamically. The lefthook.yml pre-commit hook is rewritten from a parallel lint/checkType pair to a sequential jobs pipeline with per-tool auto-staging.

Changes

Resource-aware lint execution and pre-commit pipeline

Layer / File(s) Summary
Resource thresholds and serial/parallel runner helpers
scripts/vsh/src/lint/index.ts
Imports availableParallelism and freemem from Node OS, defines CPU_CORE_THRESHOLD and free-memory threshold constants, and adds runSerial (collects failures sequentially) and runParallel (starts all subprocesses concurrently, kills siblings on failure).
runLint thread count and execution mode
scripts/vsh/src/lint/index.ts
Refactors runLint to compute defaultThreads (4 or 2) from CPU/memory thresholds, derive threadsArg from an explicit --threads flag or defaultThreads, build a commands list, and dispatch to runSerial or runParallel based on CPU core count.
Pre-commit hook sequential jobs pipeline
lefthook.yml
Replaces the prior parallel: true lint/checkType setup with a sequential jobs list: oxlint, oxfmt, eslint, and stylelint each run --fix with stage_fixed: true over staged files, followed by checkType running pnpm check:type.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • vbenjs/vue-vben-admin#6104: Directly introduces the lefthook.yml pre-commit hook that this PR refactors into a sequential jobs pipeline.
  • vbenjs/vue-vben-admin#7929: Modifies the same lefthook.yml pre-commit configuration and scripts/vsh/src/lint/index.ts thread-handling logic that this PR changes.

Suggested reviewers

  • anncwb
  • vince292007
  • mynetfan
  • jinmao88

Poem

🐇 Hop hop, the linter runs in line,
No more chaos — jobs align!
CPU cores decide the pace,
Serial or parallel, each finds its place.
stage_fixed: true means fixes stick,
Clean commits done double-quick! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete; it lacks proper checkbox selections and detailed explanations required by the template. Only the bullet points provide context about changes. Select appropriate type of change checkboxes (at least 'Bug fix' appears relevant), check relevant checklist items completed, and provide a formal description section summarizing the changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title describes the main changes: optimizing lint execution strategy and fixing pre-commit dependency resolution, which aligns with the changeset modifications to lefthook.yml and lint/index.ts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

scripts/vsh/src/lint/index.ts

ESLint 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 45399d0 and dc3865f.

📒 Files selected for processing (2)
  • lefthook.yml
  • scripts/vsh/src/lint/index.ts

Comment thread lefthook.yml
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}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n scripts/vsh/src/lint/index.ts | head -120 | tail -40

Repository: vbenjs/vue-vben-admin

Length of output: 1455


🏁 Script executed:

rg -A 20 "LintCommandOptions" scripts/vsh/src/lint/index.ts

Repository: vbenjs/vue-vben-admin

Length of output: 1179


🏁 Script executed:

rg -B 10 "const threadsArg" scripts/vsh/src/lint/index.ts

Repository: vbenjs/vue-vben-admin

Length of output: 458


🏁 Script executed:

rg "threads" scripts/vsh/src/lint/index.ts -A 2 -B 2

Repository: vbenjs/vue-vben-admin

Length of output: 1178


🏁 Script executed:

rg -B 30 "\.option\('--threads" scripts/vsh/src/lint/index.ts | head -50

Repository: vbenjs/vue-vben-admin

Length of output: 874


🏁 Script executed:

rg "\.action\(" scripts/vsh/src/lint/index.ts -A 5 -B 5

Repository: vbenjs/vue-vben-admin

Length of output: 319


🏁 Script executed:

rg "import.*cac" scripts/vsh/src/lint/index.ts

Repository: 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 -30

Repository: vbenjs/vue-vben-admin

Length of output: 1008


🏁 Script executed:

rg "cac\|CAC" scripts/vsh/src/lint/index.ts -B 5 -A 15

Repository: vbenjs/vue-vben-admin

Length of output: 47


🏁 Script executed:

fd "cac" scripts/vsh/src --type f | head -5

Repository: vbenjs/vue-vben-admin

Length of output: 47


🏁 Script executed:

rg "cac.*parse\|\.parse\(" scripts/vsh/src -A 5 -B 5 | head -40

Repository: vbenjs/vue-vben-admin

Length of output: 47


🏁 Script executed:

find scripts/vsh/src -name "*.ts" -type f | head -20

Repository: vbenjs/vue-vben-admin

Length of output: 268


🏁 Script executed:

rg "defineLintCommand" scripts/vsh/src -B 5 -A 5

Repository: vbenjs/vue-vben-admin

Length of output: 2142


🏁 Script executed:

rg "\.parse\(" scripts/vsh/src/bin -A 10 -B 5

Repository: vbenjs/vue-vben-admin

Length of output: 125


🏁 Script executed:

cat -n scripts/vsh/src/index.ts | tail -20

Repository: vbenjs/vue-vben-admin

Length of output: 619


🏁 Script executed:

rg "\.parse\(" scripts/vsh/src/index.ts -A 5 -B 5

Repository: vbenjs/vue-vben-admin

Length of output: 478


🏁 Script executed:

web_search
cac CLI library default behavior threads option parsing type coercion

Repository: 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:


🏁 Script executed:

rg "Number\|parseInt\|coerce" scripts/vsh/src/lint/index.ts

Repository: vbenjs/vue-vben-admin

Length of output: 47


🏁 Script executed:

cat -n scripts/vsh/src/lint/index.ts | head -100

Repository: 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.

Suggested change
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.

@jinmao88
jinmao88 merged commit dc3865f into vbenjs:main Jun 21, 2026
14 checks passed
@xingyu4j
xingyu4j deleted the lint-config branch June 30, 2026 03:22
@coderabbitai coderabbitai Bot mentioned this pull request Jul 20, 2026
16 tasks
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants