-
-
Notifications
You must be signed in to change notification settings - Fork 31
feat(types): export the LocationUrl type from virtual:uni-pages
#233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughIntroduces a new internal type alias _LocationUrl, refactors NavigateToOptions.url to use it, and adds an ambient module "virtual:uni-pages" exporting a public LocationUrl type alias. Changes apply in core declaration generation and the playground’s generated uni-pages.d.ts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Pre-merge checks (3 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/declaration.ts (1)
12-16: Bug: tabBar pages aren’t excluded from NavigateToOptions due to quoted-vs-unquoted mismatch.
tabsPagesPathholds quoted literals (e.g., '"/pages/index"'), but you compare againstpage.path(e.g., 'pages/index'). This causes tab pages to leak into_LocationUrl, allowingnavigateToto tab pages.Apply a two-list (unquoted for set ops, quoted for emission) fix:
- const tabsPagesPath = ctx.pagesGlobConfig?.tabBar?.list?.map((v) => { - return `"/${v!.pagePath}"` - }) ?? [] - const allPagesPath = [...ctx.pageMetaData.filter(page => !tabsPagesPath.includes(page.path)).map(v => `"/${v.path}"`), ...subPagesPath] + const tabPaths = ctx.pagesGlobConfig?.tabBar?.list?.map(v => v!.pagePath) ?? [] + const tabsPagesPath = tabPaths.map(p => `"/${p}"`) + const tabPathSet = new Set(tabPaths) + const allPagesPath = [ + ...ctx.pageMetaData + .filter(page => !tabPathSet.has(page.path)) + .map(v => `"/${v.path}"`), + ...subPagesPath, + ]
🧹 Nitpick comments (4)
packages/playground/src/uni-pages.d.ts (1)
6-26: Avoid leaking a global helper type; scope it to a namespace.
_LocationUrlat global scope can collide in consumer projects. Consider scoping and referencing via a prefixed namespace.-type _LocationUrl = - "/pages/index" | - "/pages/A-top" | - "/pages/i18n" | - "/pages/test-json" | - "/pages/test-jsonc-with-comment" | - "/pages/test-yaml" | - "/pages/test" | - "/pages/blog/index" | - "/pages/blog/post" | - "/pages/define-page/async-function" | - "/pages/define-page/conditional-compilation" | - "/pages/define-page/function" | - "/pages/define-page/nested-function" | - "/pages/define-page/object" | - "/pages/define-page/option-api" | - "/pages/define-page/remove-console" | - "/pages/define-page/yaml" | - "/pages-sub/index" | - "/pages-sub/about/index" | - "/pages-sub/about/your"; +declare namespace UniPagesInternal { + export type LocationUrl = + "/pages/index" | + "/pages/A-top" | + "/pages/i18n" | + "/pages/test-json" | + "/pages/test-jsonc-with-comment" | + "/pages/test-yaml" | + "/pages/test" | + "/pages/blog/index" | + "/pages/blog/post" | + "/pages/define-page/async-function" | + "/pages/define-page/conditional-compilation" | + "/pages/define-page/function" | + "/pages/define-page/nested-function" | + "/pages/define-page/object" | + "/pages/define-page/option-api" | + "/pages/define-page/remove-console" | + "/pages/define-page/yaml" | + "/pages-sub/index" | + "/pages-sub/about/index" | + "/pages-sub/about/your"; +} interface NavigateToOptions { - url: _LocationUrl; + url: UniPagesInternal.LocationUrl; } declare module "virtual:uni-pages" { - export type LocationUrl = _LocationUrl; + export type LocationUrl = UniPagesInternal.LocationUrl; }Also applies to: 28-30, 46-48
packages/core/src/declaration.ts (3)
29-31: Terminate generated property with a semicolon.Minor parsing/style improvement for the emitted interface.
- ${tabsPagesPath.length ? `url: ${tabsPagesPath.join(' | ')}` : ''} + ${tabsPagesPath.length ? `url: ${tabsPagesPath.join(' | ')};` : ''}
21-23: Optionally avoid a global_LocationUrlby namespacing the helper type.Prevents polluting consumer global types while keeping external API the same.
-type _LocationUrl = - ${allPagesPath.join(' |\n ')}; +declare namespace UniPagesInternal { + export type LocationUrl = + ${allPagesPath.join(' |\n ')}; +} interface NavigateToOptions { - url: _LocationUrl; + url: UniPagesInternal.LocationUrl; } declare module "virtual:uni-pages" { - export type LocationUrl = _LocationUrl; + export type LocationUrl = UniPagesInternal.LocationUrl; }Also applies to: 25-25, 42-44
42-44: Add a short doc hint to encourageimport type.Helps avoid value import mistakes in strict TS configs.
declare module "virtual:uni-pages" { - export type LocationUrl = _LocationUrl; + /** + * Use with: `import type { LocationUrl } from 'virtual:uni-pages'` + */ + export type LocationUrl = _LocationUrl; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/core/src/declaration.ts(2 hunks)packages/playground/src/uni-pages.d.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-28T03:00:09.715Z
Learnt from: xiaohe0601
PR: uni-helper/vite-plugin-uni-pages#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/src/uni-pages.d.ts
🧬 Code graph analysis (1)
packages/core/src/declaration.ts (2)
packages/core/src/context.ts (2)
PageContext(29-377)virtualModule(356-360)packages/core/src/index.ts (1)
VitePluginUniPages(47-156)
🔇 Additional comments (2)
packages/playground/src/uni-pages.d.ts (1)
46-48: LGTM: public LocationUrl export matches PR goal.The ambient module cleanly exposes
LocationUrlfor externalimport typeusage. Nice.packages/core/src/declaration.ts (1)
8-47: TabBar exclusion logic verified
packages/playground/src/uni-pages.d.tsdefines_LocationUrlas"/pages/index"and"/pages/A-top"andSwitchTabOptionshas nourlproperty, confirming no tabBar pages leaked into the union.
Description 描述
从
virtual:uni-pages导出LocationUrl类型,可以提供给外部使用。Linked Issues 关联的 Issues
fix #206
Summary by CodeRabbit
New Features
Refactor