fix: 处理exclude 配置不生效和不够健壮问题 #260
Conversation
分离排除规则中的否定模式（以!开头的规则），先使用肯定模式过滤文件，再单独处理否定模式将应包含的文件添加回结果集，确保文件筛选逻辑正确
添加三个测试用例，分别测试： 1. 排除所有 pages 下的 components 目录 2. 排除 pages 下的 components 目录但保留 pages-sub2 下的 components 3. 基本文件获取功能
在 vite.config.ts 中添加 exclude 配置，用于排除 pages 目录下的 components 文件夹，同时保留 pages-sub2 中的 components 文件
WalkthroughThe PR modifies Changes
Sequence DiagramsequenceDiagram
participant caller as Caller
participant glob as Glob Engine
participant merge as Dedup Merge
rect rgb(220, 240, 255)
Note over caller,glob: Old Flow (Single Glob)
caller->>glob: glob(pattern, {ignore: excludePatterns})
glob->>caller: files (excludes applied uniformly)
end
rect rgb(240, 255, 220)
Note over caller,merge: New Flow (Two-Step with Negation)
caller->>glob: Split excludes into positive & negative
caller->>glob: glob(pattern, {ignore: positivePatterns})
glob->>caller: result1 (initial files)
alt if negativePatterns exist
caller->>glob: glob(negativePatterns)
glob->>caller: result2 (re-include candidates)
caller->>merge: merge(result1, result2)
merge->>caller: final (deduplicated)
else
caller->>caller: return result1
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related issues
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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 (7)
packages/playground/src/pages-sub2/about/your.vue (1)
1-7: LGTM! Test fixture is appropriate.This minimal Vue component serves its purpose as a test fixture to validate the negated exclude pattern functionality introduced in this PR.
Optional refinement: The empty
<script>block on line 1 could be removed entirely or replaced with<script setup>if future logic is needed, but for a test fixture this is perfectly fine as-is.packages/playground/vite.config.ts (1)
20-23: Exclude/negation config looks correct; confirm cwd and document behavior.
- Patterns assume cwd is the project src root used by file discovery. Please verify the plugin scans with cwd='packages/playground/src' so these patterns match as intended.
- Suggest adding a brief README note: exclude supports negation ('!') and later entries can re-include earlier exclusions; order matters.
packages/core/src/files.ts (2)
16-20: Clarify naming and intent of pattern groups.positivePatterns actually hold the “exclude” rules (non-! entries), and negativePatterns are the “re-include” rules. Consider renaming to excludePatterns and reincludePatterns, and adjust comments accordingly for future maintainability.
12-13: Defensive default for exclude.If ResolvedOptions.exclude can be undefined, default it to [] to avoid surprises.
- const { exclude } = options + const { exclude = [] } = optionstest/files.spec.ts (3)
9-9: Remove unnecessaryasynckeyword.The test function is marked
asyncbut contains noawaitstatements or promise handling.Apply this diff:
- it('pages', async () => { + it('pages', () => {
65-65: Remove unnecessaryasynckeyword.The test function is marked
asyncbut contains noawaitstatements or promise handling.Apply this diff:
- it('pages2', async () => { + it('pages2', () => {
115-115: Remove unnecessaryasynckeyword.The test function is marked
asyncbut contains noawaitstatements or promise handling.Apply this diff:
- it('pages3', async () => { + it('pages3', () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
packages/core/src/files.ts(1 hunks)packages/playground/src/pages-sub/about/components/comp.vue(1 hunks)packages/playground/src/pages-sub/about/components/dir/nest-comp.vue(1 hunks)packages/playground/src/pages-sub/components/comp.vue(1 hunks)packages/playground/src/pages-sub/components/dir/nest-comp.vue(1 hunks)packages/playground/src/pages-sub2/about/components/comp.vue(1 hunks)packages/playground/src/pages-sub2/about/components/dir/nest-comp.vue(1 hunks)packages/playground/src/pages-sub2/about/index.vue(1 hunks)packages/playground/src/pages-sub2/about/your.vue(1 hunks)packages/playground/src/pages-sub2/components/comp.vue(1 hunks)packages/playground/src/pages-sub2/components/dir/nest-comp.vue(1 hunks)packages/playground/src/pages-sub2/index.vue(1 hunks)packages/playground/src/pages/blog/components/comp.vue(1 hunks)packages/playground/src/pages/blog/components/dir/nest-comp.vue(1 hunks)packages/playground/src/pages/components/comp.vue(1 hunks)packages/playground/src/pages/components/dir/nest-comp.vue(1 hunks)packages/playground/vite.config.ts(1 hunks)test/files.spec.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-28T03:00:09.715Z
Learnt from: xiaohe0601
Repo: uni-helper/vite-plugin-uni-pages PR: 229
File: packages/core/client.d.ts:9-11
Timestamp: 2025-08-28T03:00:09.715Z
Learning: In uni-helper/vite-plugin-uni-pages, global type declarations like `definePage` are kept in a separate `client.d.ts` file and exposed via the `./client` export. Users need to manually add `uni-helper/vite-plugin-uni-pages/client` to their tsconfig.json to access these global declarations, which is documented as the intended usage pattern.
Applied to files:
packages/playground/vite.config.tspackages/playground/src/pages-sub2/index.vue
🧬 Code graph analysis (1)
test/files.spec.ts (2)
packages/core/src/options.ts (1)
resolveOptions(9-65)packages/core/src/files.ts (1)
getPageFiles(11-42)
🔇 Additional comments (18)
packages/playground/src/pages-sub/about/components/dir/nest-comp.vue (1)
1-5: LGTM: template-only stub is fine for playground tests.packages/playground/src/pages-sub2/about/index.vue (1)
1-7: LGTM: minimal page stub fits the test scaffolding.packages/playground/src/pages/components/dir/nest-comp.vue (1)
1-5: LGTM: static SFC for exclude-rule coverage.packages/playground/src/pages/blog/components/dir/nest-comp.vue (1)
1-5: LGTM: consistent with other playground stubs.packages/playground/src/pages/components/comp.vue (1)
1-5: LGTM.packages/playground/src/pages-sub/components/comp.vue (1)
1-5: LGTM: suitable for exclude/negation test matrix.packages/playground/src/pages-sub2/about/components/dir/nest-comp.vue (1)
1-5: LGTM: aligns with the “!pages-sub2//components//.” negation case.packages/playground/src/pages-sub/about/components/comp.vue (1)
1-5: LGTM.packages/playground/src/pages-sub2/about/components/comp.vue (1)
1-5: LGTM for fixture SFC.Minimal template is fine for the playground fixture.
packages/playground/src/pages/blog/components/comp.vue (1)
1-5: LGTM for fixture SFC.Good as a discovery target in tests.
packages/playground/src/pages-sub2/components/dir/nest-comp.vue (1)
1-5: LGTM for fixture SFC.No issues.
packages/playground/src/pages-sub/components/dir/nest-comp.vue (1)
1-5: LGTM for fixture SFC.Works for discovery tests.
packages/playground/src/pages-sub2/components/comp.vue (1)
1-5: LGTM for fixture SFC.Consistent with the other test components.
packages/playground/src/pages-sub2/index.vue (1)
1-60: LGTM for playground page.Simple nav and route meta are fine for the fixture.
test/files.spec.ts (4)
5-8: LGTM! Test scope broadened to validate cross-directory exclusion.The change from
packages/playground/src/pagestopackages/playground/srcenables testing the negation pattern feature across multiple page directories (pages,pages-sub,pages-sub2, etc.), which is essential for validating the PR's objectives.
9-59: Comprehensive baseline test validates default behavior.The snapshot correctly captures all files including component files from all directories when no custom exclude patterns are applied. This serves as an effective baseline for comparing the filtered results in subsequent tests.
61-103: Test correctly validates blanket component exclusion.The snapshot accurately reflects the behavior of the
pages*/**/components/**/*.*pattern: all component files withinpages,pages-sub,pages-sub2, andpages-blogdirectories are excluded, whilecomponents/Counter.vue(which doesn't match thepages*prefix) is correctly retained.
105-157: Excellent test coverage for the negation pattern feature!This test validates the core functionality of the PR: the negation pattern
!pages-sub2/**/components/**/*.*correctly overrides the broader exclusionpages*/**/components/**/*.*, selectively re-including only thepages-sub2component files while keeping other component directories excluded. The snapshot accurately reflects this behavior.
| const includedFiles = fg.sync(negativePatterns.map(pattern => `${pattern}`), { | ||
| onlyFiles: true, | ||
| cwd: path, | ||
| }) | ||
|
|
||
| // 将应该包含的文件添加回结果集，确保不重复 | ||
| const filesSet = new Set([...files, ...includedFiles]) | ||
| files = Array.from(filesSet) | ||
| } |
There was a problem hiding this comment.
Bug: re-included files are not filtered by allowed extensions.
negativePatterns are globbed without extension filtering, so non-page files (e.g., images, JSON) can leak into results. Filter by FILE_EXTENSIONS before merging and ensure deterministic ordering.
Apply this patch:
- const includedFiles = fg.sync(negativePatterns.map(pattern => `${pattern}`), {
- onlyFiles: true,
- cwd: path,
- })
-
- // 将应该包含的文件添加回结果集，确保不重复
- const filesSet = new Set([...files, ...includedFiles])
- files = Array.from(filesSet)
+ const includedFiles = fg.sync(negativePatterns, {
+ onlyFiles: true,
+ cwd: path,
+ })
+ // 仅保留受支持的页面后缀
+ const includedPageFiles = includedFiles.filter(f =>
+ FILE_EXTENSIONS.some(ext => f.endsWith(`.${ext}`)),
+ )
+ // 合并并保持结果稳定（去重 + 排序）
+ const filesSet = new Set([...files, ...includedPageFiles])
+ files = Array.from(filesSet).sort()Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/core/src/files.ts around lines 31 to 39, the code re-includes files
matched by negativePatterns without filtering by allowed FILE_EXTENSIONS and
without deterministic ordering; update the logic so includedFiles are filtered
to only keep files whose extensions are in the FILE_EXTENSIONS list (use the
file extension check, e.g., via path.extname and normalizing the extension)
before merging into filesSet, then produce the final files array by converting
filesSet to an array and sorting it (deterministic ordering) so duplicates are
removed, only allowed extensions are included, and ordering is stable.
1 participant
本次提交不仅支持普通的exclude过滤，还支持!排除项，使用示例如下：(测试用例通过）
Summary by CodeRabbit
New Features
Tests
Chores