feat: 实现指定页面排除根组件的功能#21
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds an Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Vite as Vite
participant Plugin as UniKuRoot
participant Utils as loadPagesJson
participant FS as File Watcher
Dev->>Vite: start dev / build
Vite->>Plugin: initialize(options { excludePages? })
Plugin->>Utils: loadPagesJson(pages.json, root)
Utils-->>Plugin: pages (string[])
alt excludePages provided
Plugin->>Plugin: createFilter(pages, options.excludePages)
else no excludePages
Plugin->>Plugin: createFilter(pages)
end
Plugin->>Vite: apply transforms using filtered pages
note over FS,Plugin: watch pages.json / pages files
FS-->>Plugin: file change event
Plugin->>Utils: reload pagesJson
Utils-->>Plugin: pages
Plugin->>Plugin: reapply excludePages filter if present
Plugin->>Vite: refresh transforms
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
Comment |
|
@cnguu 我觉得可以考虑增加用户体验 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils.ts (1)
28-38: Fix unsafe parsing and incorrect default leading to runtime errors
jsonParse(pagesJsonRaw)may returnundefinedon invalid JSONC; destructuring then throws.pages = {}default is incorrect;{}.mapwill throw. Should be an array.Apply:
-export function loadPagesJson(path: string, rootPath: string): string[] { - const pagesJsonRaw = readFileSync(path, 'utf-8') - - const { pages = [], subPackages = [] } = jsonParse(pagesJsonRaw) - - return [ - ...pages - .map((page: any) => formatPagePath(rootPath, page.path)), - ...subPackages - .map(({ pages = {}, root = '' }: any) => { - return pages.map((page: any) => formatPagePath(join(rootPath, root), page.path)) - }) - .flat(), - ] -} +export function loadPagesJson(path: string, rootPath: string): string[] { + const pagesJsonRaw = readFileSync(path, 'utf-8') + const parsed = jsonParse(pagesJsonRaw) || {} + + const pages = Array.isArray(parsed.pages) ? parsed.pages : [] + const subPackages = Array.isArray(parsed.subPackages) ? parsed.subPackages : [] + + return [ + ...pages.map((page: { path: string }) => formatPagePath(rootPath, page.path)), + ...subPackages.flatMap( + ({ pages = [], root = '' }: { pages?: { path: string }[]; root?: string }) => + (Array.isArray(pages) ? pages : []).map((page) => + formatPagePath(join(rootPath, root!), page.path), + ), + ), + ] +}
🧹 Nitpick comments (6)
.gitignore (1)
6-7: Consider scoping and a few common extras
- Prefer directory form to avoid matching files named
dist:dist/- Optional: add
*.log,coverage/,.DS_Store, and*.localif relevant.- dist + dist/ + coverage/ + *.log + .DS_Storeexamples/src/pages/excluded.vue (1)
8-10: Avoid returning from the click handler
showToast()/hideToast()don’t return meaningful values; returning is unnecessary.-function handleClick() { - return globalToastState.value ? hideToast() : showToast() -} +function handleClick() { + globalToastState.value ? hideToast() : showToast() +}README.md (1)
497-524: Tighten the filter example to reduce false positives and clarify inputUse
endsWith(paths are normalized and absolute.vuefile paths) to avoid accidental matches.- // filterPage?: (pagePaths: string[]) => string[] - filterPage(pagePaths) { - return pagePaths.filter(path => !path.includes('excluded.vue')) - }, + // filterPage?: (pagePaths: string[]) => string[] + // pagePaths are normalized absolute .vue file paths + filterPage(pagePaths) { + return pagePaths.filter(p => !p.endsWith('/excluded.vue')) + },examples/vite.config.ts (1)
15-17: Example works; add a tiny TS annotation for clarity.Explicitly type the param to self-document the API.
- filterPage(pagePaths) { + filterPage(pagePaths: string[]) { return pagePaths.filter(path => !path.includes('excluded.vue')) },src/index.ts (2)
31-35: Public hook addition looks good; tighten the JSDoc.Recommend documenting that:
- Input is the raw pages list from pages.json (same format as loadPagesJson).
- Must return a string[] subset.
- Called at startup and on pages.json add/change.
- Should be pure and not mutate the input array.
80-83: Avoid name shadowing with the new option; also cache the filter to avoid per-transform allocation.Rename the local predicate and precompute it once, updating on watch.
- const filterPage = createFilter(pagesJson) - if (filterPage(pageId)) { + const pageIdFilter = createFilter(pagesJson) + if (pageIdFilter(pageId)) { ms = await transformPage(code, options.enabledGlobalRef) }Further improvement (optional): hoist and cache
- let pagesJson = applyFilterPage(rawPagesJson) + let pagesJson = applyFilterPage(rawPagesJson) + let pageIdFilter = createFilter(pagesJson) ... - pagesJson = applyFilterPage(rawPagesJson) + pagesJson = applyFilterPage(rawPagesJson) + pageIdFilter = createFilter(pagesJson) ... - const pageIdFilter = createFilter(pagesJson) - if (pageIdFilter(pageId)) { + if (pageIdFilter(pageId)) { ms = await transformPage(code, options.enabledGlobalRef) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)README.md(1 hunks)examples/src/pages.json(1 hunks)examples/src/pages/excluded.vue(1 hunks)examples/vite.config.ts(1 hunks)src/index.ts(3 hunks)src/utils.ts(1 hunks)
🔇 Additional comments (3)
.gitignore (1)
6-7: LGTM on new ignoresAdding
distand.ideais helpful for keeping the repo clean.src/utils.ts (1)
25-25: Explicit return type is goodTyping
loadPagesJson(...): string[]aligns with downstream filtering.examples/src/pages.json (1)
20-26: Demo page entry looks correctThe new
pages/excludedentry is valid JSON and matches the README demo.
|
@skiyee 更新了,换成 vite 核心支持的方式 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
examples/vite.config.ts (1)
15-15: Clarify config naming: PR mentionsfilterPage, example usesexclude. Align or show both.If both are supported, consider demonstrating both in the example; if only one is supported, update the PR description/README accordingly.
Example (either/or):
UniKuRoot({ // Option A: glob/regex pattern(s) exclude: '**/excluded.vue', // exclude: [/excluded\.vue$/, '**/drafts/**'], // Option B: functional filter // filterPage: (paths) => paths.filter(p => !p.includes('excluded.vue')), })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)examples/vite.config.ts(1 hunks)src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/index.ts
- README.md
|
👋 谢谢PR,请问可以简单描述一下,有哪些不使用根组件的示例? |
|
@skiyee 一些中转页面,一些webview页面,一些独立访问的页面,这些都不适合走根组件的生命周期,减少不必要的判断和打包后的大小 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/index.ts (2)
82-85: Wire alias, make glob resolution robust, and handle ? query in Vite ids.
- Use exclude ?? excludePages.
- Pass { resolve: rootPath } so relative globs work consistently.
- Strip query from id before filtering.
- const filterPage = createFilter(pagesJson, options.excludePages) - if (filterPage(pageId)) { + const exclude = options.exclude ?? options.excludePages + const filterPage = createFilter(pagesJson, exclude, { resolve: rootPath }) + const cleanId = pageId.split('?', 1)[0] + if (filterPage(cleanId)) { ms = await transformPage(code, options.enabledGlobalRef) }
3-3: Import FilterPattern from @rollup/pluginutils
src/index.ts:3
Replace theFilterPatternimport from 'vite' with one from '@rollup/pluginutils' to ensure compatibility across Vite (v4/v5) and Rollup.-import type { FilterPattern, Plugin } from 'vite' +import type { Plugin } from 'vite' +import type { FilterPattern } from '@rollup/pluginutils'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
examples/vite.config.ts(1 hunks)src/index.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/vite.config.ts
|
THANK YOU ❤ |
总会有一些不需要根组件的页面,增加
exclude选项来过滤掉Summary by CodeRabbit
New Features
Documentation
Examples
Chores