Skip to content

Conversation

@xiaohe0601
Copy link
Member

@xiaohe0601 xiaohe0601 commented Sep 10, 2025

Description 描述

virtual:uni-pages 导出 LocationUrl 类型,可以提供给外部使用。

import type { LocationUrl } from 'virtual:uni-pages'

const url: LocationUrl = '/pages/index' // ✅
const url: LocationUrl = '/pages/1ndex' // ❌

Linked Issues 关联的 Issues

fix #206

Summary by CodeRabbit

  • New Features

    • Added an exported LocationUrl type for page URLs via the virtual module, enabling clearer URL typings across the app.
  • Refactor

    • Updated NavigateToOptions.url to use the new LocationUrl type alias instead of a long string-literal union.
    • Introduced an internal URL type alias to centralize allowed page paths and reduce duplication in type declarations.

@coderabbitai
Copy link

coderabbitai bot commented Sep 10, 2025

Walkthrough

Introduces 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

Cohort / File(s) Summary
Core type declarations
packages/core/src/declaration.ts
Add internal type alias _LocationUrl (union of page paths); change NavigateToOptions.url to _LocationUrl; declare ambient module "virtual:uni-pages" exporting type LocationUrl = _LocationUrl.
Playground generated d.ts
packages/playground/src/uni-pages.d.ts
Add global _LocationUrl; update NavigateToOptions.url to _LocationUrl; add declare module "virtual:uni-pages" exporting type LocationUrl = _LocationUrl.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

size/XS

Poem

In burrows of types where paths entwine,
I nibble new aliases, sleek and fine.
A hop to Navigate, URLs in tow—
One name to rule where pages go.
Now exports bloom like clover in spring,
Type-safe trails for every wing. 🐇✨

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.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

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)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning The PR exports only the LocationUrl type but does not add export statements for other generated interfaces such as NavigateToOptions as requested by issue #206, so it does not fully satisfy the requirement to make plugin-generated types available for external import. Please add export declarations for all generated types, including NavigateToOptions and any other relevant interfaces in the uni-pages.d.ts module, to fully address issue #206.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely describes the main change by indicating that the PR exports the LocationUrl type from the virtual module, matching the primary objective of the pull request without extraneous detail.
Out of Scope Changes Check ✅ Passed All modifications in both the core declaration generator and the generated d.ts files directly support the export of the LocationUrl type and the refactoring of NavigateToOptions, with no unrelated or extraneous changes introduced.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link

@coderabbitai coderabbitai bot left a 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.

tabsPagesPath holds quoted literals (e.g., '"/pages/index"'), but you compare against page.path (e.g., 'pages/index'). This causes tab pages to leak into _LocationUrl, allowing navigateTo to 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.

_LocationUrl at 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 _LocationUrl by 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 encourage import 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

📥 Commits

Reviewing files that changed from the base of the PR and between f42e1b4 and f440ec0.

📒 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 LocationUrl for external import type usage. Nice.

packages/core/src/declaration.ts (1)

8-47: TabBar exclusion logic verified

packages/playground/src/uni-pages.d.ts defines _LocationUrl as "/pages/index" and "/pages/A-top" and SwitchTabOptions has no url property, confirming no tabBar pages leaked into the union.

@FliPPeDround FliPPeDround merged commit 591bbea into uni-helper:main Sep 12, 2025
4 checks passed
@xiaohe0601 xiaohe0601 deleted the feat/types branch September 17, 2025 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

新增类型导出

2 participants