feat: filter pages in mergePageMetaData to exclude sub-package pages #258#259
feat: filter pages in mergePageMetaData to exclude sub-package pages #258#259FliPPeDround merged 4 commits intouni-helper:mainfrom
Conversation
WalkthroughMain change: sub-package pages are scanned and their absolute paths are removed from the main pages map before main page metadata is generated; playground is updated with a sub-package demo, config, and type entry. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test
participant PC as PageContext
participant FS as FileSystem
participant Merge as mergePageMetaData
Note left of Test: Setup with subPackages
Test->>PC: new PageContext({ dir, subPackages: [...] })
Test->>PC: await PC.scanSubPages()
PC->>FS: read sub-package page files
FS-->>PC: return sub-page file list (absolute paths)
PC->>Merge: prepare pages map (main + sub)
Merge->>Merge: compute sub-package roots & remove sub-page paths from main pages map
Merge->>Merge: parsePages on remaining main pages -> generate main metadata
Merge-->>PC: merged metadata (excludes sub-package pages)
Test->>PC: assertions (routes resolved)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/core/src/context.ts (1)
253-253: Hardcoded 'src/' prefix assumption may break with different directory structures.The replacement logic assumes all sub-package paths start with 'src/', which may not hold for all project configurations.
Consider normalizing paths relative to a common base instead:
- const subPages = [...this.subPages.keys()].map(v => v.replace('src/', '')) + const basePath = slash(path.join(this.options.root, this.options.outDir)) + const subPages = [...this.subPages.keys()].map(v => + slash(path.relative(basePath, path.join(this.options.root, v))) + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/core/src/context.ts(1 hunks)packages/playground/src/pages/pages-internal-sub/index.vue(1 hunks)packages/playground/src/uni-pages.d.ts(1 hunks)packages/playground/vite.config.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/uni-pages.d.ts
🧬 Code graph analysis (1)
packages/core/src/context.ts (1)
packages/core/src/page.ts (1)
Page(15-106)
🔇 Additional comments (3)
packages/playground/src/pages/pages-internal-sub/index.vue (1)
1-5: LGTM! Appropriate test case for sub-package filtering.This simple Vue component effectively serves as a test case for the new sub-package filtering functionality.
packages/playground/vite.config.ts (1)
19-19: LGTM! Appropriate test configuration.The addition of the new sub-package path correctly demonstrates the filtering functionality implemented in this PR.
packages/playground/src/uni-pages.d.ts (1)
26-27: Verify this generated file was properly regenerated.This file is marked as auto-generated (line 4), so it should be regenerated by the plugin rather than manually edited. Please confirm that this change was produced by running the plugin's type generation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/generate.spec.ts (1)
31-31: Remove commented debug code.The commented
console.logstatement appears to be leftover debug code and should be removed to keep the test file clean.Apply this diff:
- // console.log(ctx.pages)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/context.ts(1 hunks)test/files.spec.ts(1 hunks)test/generate.spec.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/context.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/generate.spec.ts (1)
packages/core/src/context.ts (1)
PageContext(31-503)
🔇 Additional comments (3)
test/files.spec.ts (1)
27-27: LGTM! Snapshot correctly reflects sub-package page discovery.The addition of
"pages-internal-sub/index.vue"to the snapshot is correct. ThegetPageFilesfunction scans the pages directory and discovers all page files, including those in sub-package directories. The filtering of sub-package pages from the main pages list occurs later inmergePageMetaData.test/generate.spec.ts (2)
26-28: LGTM! Correct test setup for sub-package filtering.The addition of
subPackagesconfiguration and thescanSubPages()call correctly set up the test to verify that sub-package pages are filtered from the main pages list. The test flow matches the implementation:scanPages()→scanSubPages()→mergePageMetaData(), wheremergePageMetaData()removes sub-package page paths from the main pages map.
179-182: LGTM! Consistent test setup for the non-merge-pages scenario.The changes mirror the first test and correctly configure sub-package scanning for the scenario where
mergePagesis false. This ensures sub-package filtering works correctly in both code paths.
|
#189 应该是同一个类型的issue,也可以关闭 |
Description 描述
close #258
close #189
Linked Issues 关联的 Issues
Additional context 额外上下文
Summary by CodeRabbit
New Features
Behavior Changes
Tests