feat: support definePage macro #223
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughRefactors page metadata into a Page class with VM-based TS execution and AST-driven definePage vs handling; adds parseSFC/execScript utilities, new types and exports, enables dev sourcemaps, adds playground definePage example pages, updates tests, and adds VS Code launch configs. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Plugin as vite-plugin (core/src/index.ts)
participant SFC as parseSFC
participant AST as Babel (findMacro)
participant Page as Page (core/src/page.ts)
Dev->>Plugin: transform(code, id)
Plugin->>Plugin: check FILE_EXTENSIONS
alt SFC file
Plugin->>SFC: parseSFC(code)
SFC-->>Plugin: descriptor
Plugin->>AST: findMacro(descriptor.script*)
alt definePage macro found
Plugin->>Plugin: remove macro by AST loc
Plugin-->>Dev: transformed code (+ sourcemap if changed)
else route block present
Plugin->>Plugin: strip <route/> custom block(s)
Plugin-->>Dev: transformed code
else
Plugin-->>Dev: no transform
end
else not SFC
Plugin-->>Dev: no transform
end
sequenceDiagram
autonumber
participant Ctx as PageContext
participant Pg as Page
participant FS as File System
participant SFC as parseSFC
participant VM as execScript
Ctx->>Pg: getPageMeta(force?)
alt cached & unchanged
Pg-->>Ctx: cached meta
else need read
Pg->>FS: read .vue file
Pg->>SFC: parseSFC(content)
SFC-->>Pg: descriptor
alt macro exists
Pg->>VM: execScript(compiled macro module)
VM-->>Pg: UserPageMeta
else route block exists
Pg->>Pg: parse route block by RouteBlockLang
else
Pg->>Pg: default page meta
end
Pg-->>Ctx: PageMetaDatum (+ hasChanged)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (26)
packages/playground/package.json (1)
17-18: Consider lazy-loadingyamlto keep bundles leanIf YAML parsing is only needed on that specific page, prefer a dynamic import to avoid pulling
yamlinto the initial bundle.Example usage in
yaml.vue(outside this file):- import { parse } from 'yaml' + // lazy-load on demand + const { parse } = await import('yaml')packages/core/build.config.ts (2)
8-8: Broaden sourcemap condition to cover non-production modesLimiting to
=== 'development'disables sourcemaps in test/CI or custom envs. Prefer enabling unless explicitly in production.- sourcemap: process.env.NODE_ENV === 'development', + sourcemap: process.env.NODE_ENV !== 'production',
1-1:processimport is unnecessaryNode exposes
processglobally; you can drop the import to reduce noise. If type errors arise, add@types/nodeto dev deps instead.-import process from 'node:process'.vscode/launch.json (2)
7-20: Auto-attach to child processes for better Node debuggingUni’s dev CLI spawns child Node processes; enable auto-attach to debug them when needed.
"runtimeArgs": ["run", "dev:h5"], "console": "integratedTerminal", "internalConsoleOptions": "neverOpen", + "autoAttachChildProcesses": true,
22-31: Enable sourcemaps in MP-Weixin debug to match H5You enabled
NODE_ENV=developmentfor H5 only. Givenbuild.config.tstoggles sourcemaps by env, mirror the setting for MP-Weixin or they’ll be off there."runtimeArgs": ["run", "dev:mp-weixin"], "console": "integratedTerminal", - "internalConsoleOptions": "neverOpen" + "internalConsoleOptions": "neverOpen", + "env": { + "NODE_ENV": "development" + }packages/core/src/types.ts (3)
147-153: Clarify precedence whenpathis provided vs. auto-generated
UserPageMeta.pathis marked deprecated and optional, but you’ll need a deterministic merge rule when both a user-specifiedpathand a derived path exist (especially for moved/renamed files).I recommend:
- On merge, prefer the derived path, and warn when a user-provided
pathdisagrees.- Document this in the JSDoc here.
If you want, I can propose a small helper that validates and logs a deprecation warning during the merge step.
155-158: SimplifyMaybePromiseCallableMinor cleanup to remove duplication and better express intent.
Apply:
-export type MaybePromise<T> = T | Promise<T> -export type MaybeCallable<T> = T | (() => T) -export type MaybePromiseCallable<T> = T | (() => T) | (() => Promise<T>) +export type MaybePromise<T> = T | Promise<T> +export type MaybeCallable<T> = T | (() => T) +export type MaybePromiseCallable<T> = T | (() => MaybePromise<T>)
159-161: Add a dev-only noop shim fordefinePageto prevent runtime ReferenceErrorsThe
definePagehelper is only a type‐level declaration and has no runtime value by itself. In environments where the macro transform doesn’t strip calls (misconfiguration, alternative toolchain, isolated AST transforms), invokingdefinePage(...)in an SFC will throw a ReferenceError. To guard against this without changing the primary macro erase path, introduce a small runtime shim in development mode.• Create a new file at
packages/core/src/runtime/shim.tscontaining:if (process.env.NODE_ENV !== 'production' && !(globalThis as any).definePage) { (globalThis as any).definePage = () => { /* no-op */ } }• Ensure this shim is loaded once on the client side. For example, in your Vite plugin (
packages/core/src/index.ts), under theconfigResolvedhook inject a virtual import of the shim:import { Plugin } from 'vite' export function VitePluginUniPages(): Plugin { return { name: 'uni-pages-shim', configResolved() { this.emitFile({ type: 'chunk', id: 'virtual:uni-pages-shim' }) }, resolveId(id) { if (id === 'virtual:uni-pages-shim') { return id } }, load(id) { if (id === 'virtual:uni-pages-shim') { return `import './runtime/shim'` } } } }• Verified that the
DefinePagetype is indeed exported from the package root viapackages/core/src/index.ts → export * from './types', soimport('.').DefinePagewill resolve correctly.This shim won’t affect production builds (stripped by
NODE_ENV), but provides a safety net during development.packages/core/client.d.ts (1)
9-11: Preferdeclare globalfor augmenting globalsUse the simpler
declare globalform (without anexport) and drop thenamespace globalThiswrapper. TheDefinePagetype is already re-exported at the package root, so yourimport('.').DefinePagewill resolve correctly.• File: packages/core/client.d.ts
• Lines: 9–11Suggested diff:
-declare namespace globalThis { - export const definePage: import('.').DefinePage -} +declare global { + const definePage: import('.').DefinePage +}I’ve confirmed that
DefinePageis defined in packages/core/src/types.ts and re-exported via src/index.ts → dist → index.d.ts at the package root, so no additional export changes are needed.packages/playground/src/pages/define-page/remove-console.vue (1)
3-3: Guard or removeconsole.logIf this page demonstrates stripping console calls, consider guarding the log for dev only to keep prod bundles clean, and still allow interactive testing.
Apply:
- console.log('abc ...'); // this is console should be remove + if (import.meta.env.DEV) console.log('abc ...')packages/playground/src/pages/define-page/yaml.vue (1)
5-10: Optional: add typing or error handling around YAML parsing
yaml.parsereturnsany, so typos won’t be caught (e.g.,navigationBarTitletext). If convenient in the playground, assert or validate the shape to catch mistakes early.Options:
- Lightweight assertion:
- return yamlParser(yml);
- return yamlParser(yml) as any; // consider narrowing to UserPageMeta if types are importable
- Or, if you can import the type in the playground: ```ts import type { UserPageMeta } from 'vite-plugin-uni-pages' // adjust path as needed const meta = yamlParser(yml) as UserPageMeta return meta
- Or validate at runtime with a minimal guard before returning. I can draft a tiny validator if you’d like.
packages/core/index.d.ts (1)
1-1: Ensure global augmentation is actually published to consumersThe triple-slash path reference is fine locally, but only effective if client.d.ts is included in the published package. Verify package.json "files" (or export map) includes both index.d.ts and client.d.ts so global definePage is visible to downstream projects.
If needed, add client.d.ts to the published files. Example patch (outside this file):
# packages/core/package.json { "name": "@uni-helper/uni-pages-core", + "types": "./index.d.ts", + "files": [ + "dist", + "index.d.ts", + "client.d.ts" + ], "exports": { ".": { "types": "./index.d.ts", "default": "./dist/index.js" } } }packages/playground/src/pages/define-page/async-function.vue (2)
3-6: Avoid non-deterministic titles in a build-time macroIf definePage is evaluated at build time, Math.random() can cause flaky outputs, cache misses, and hard-to-reproduce diffs. Prefer a deterministic example while still demonstrating async.
Apply:
- const randomTxt = await new Promise<string>((resolve) => { - const txt = Math.random().toString(36).slice(-8); - resolve(txt); - }); + // Demonstrate async without nondeterminism + const randomTxt = await Promise.resolve('Async definePage example');
19-21: Use platform-friendly tags for uni-app targetsIf this playground runs across H5/APP/MP, prefer over
to avoid runtime/platform inconsistencies.- <div>test</div> + <view>test</view>packages/playground/src/pages/define-page/function.vue (2)
5-13: Prefersatisfies+ literal inference for tighter type checksUsing a type annotation on words widens literal types. Using satisfies with a const assertion preserves literals and still checks shape. Minor readability win to switch to a template literal for the title.
- const words: HelloWorld = { - hello: 'hello', - world: 'world', - }; + const words = { + hello: 'hello', + world: 'world', + } as const satisfies HelloWorld; return { style: { - navigationBarTitleText: [words.hello, words.world].join(' '), + navigationBarTitleText: `${words.hello} ${words.world}`, },
21-23: Consistent cross-platform tagSame note as other pages: consider for broader uni-app compatibility.
- <div>define page in function mode</div> + <view>define page in function mode</view>packages/playground/src/pages/define-page/nested-function.vue (1)
21-23: Cross-platform tag consistency (optional)Mirror the recommendation for consistency across the define-page examples.
- <div>test</div> + <view>test</view>packages/playground/src/pages/define-page/option-api.vue (2)
4-8: Placement of definePage is correct; add a semicolon and align formattingTop-level invocation before export default is correct for Option API SFCs. Add a trailing semicolon for consistency with the rest of the repo’s style.
definePage({ style: { navigationBarTitleText: 'Option API 内使用 definePage' } -}) +});
1-13: Optional: add a minimal template to avoid empty-render warningsSome targets warn on pages without a template. A minimal stub keeps the example complete.
</script> +<template> + <view /> +</template>packages/core/src/index.ts (2)
86-88: Consider using a Set for better performance.Using
Array.find()for file extension checking is less efficient than a Set lookup, especially when called frequently during transformation.+const FILE_EXTENSIONS_SET = new Set(FILE_EXTENSIONS) // ... -if (!FILE_EXTENSIONS.find(ext => id.endsWith(ext))) { +const ext = id.split('.').pop() +if (!ext || !FILE_EXTENSIONS_SET.has(ext)) { return null }
115-124: Consider more robust route block removal.The regex pattern for removing route blocks might not handle edge cases like nested tags or unusual whitespace. Also, the removal could be more efficient using the parsed SFC structure.
Since you already have the route block from the parsed SFC, consider using its position information for removal:
if (routeBlock) { - const routeBlockMatches = s.original.matchAll( - /<route[^>]*>([\s\S]*?)<\/route>/g, - ) - for (const match of routeBlockMatches) { - const index = match.index! - const length = match[0].length - s.remove(index, index + length) - } + // Use the SFC parser's position information for accurate removal + const { loc } = routeBlock + if (loc && loc.start && loc.end) { + s.remove(loc.start.offset, loc.end.offset) + } else { + // Fallback to regex if position info is missing + const routeBlockMatches = s.original.matchAll( + /<route[^>]*>([\s\S]*?)<\/route>/g, + ) + for (const match of routeBlockMatches) { + const index = match.index! + const length = match[0].length + s.remove(index, index + length) + } + } }packages/core/src/context.ts (2)
91-98: Consider extracting page creation logic to reduce duplication.The pattern of checking existing pages and creating new ones is duplicated between
scanPagesandscanSubPages.+private getOrCreatePage(path: PagePath, existingMap?: Map<string, Page>): Page { + return existingMap?.get(path.absolutePath) || new Page(this, path) +} async scanPages() { // ... existing code ... const pages = new Map<string, Page>() for (const path of paths) { - const page = this.pages.get(path.absolutePath) || new Page(this, path) + const page = this.getOrCreatePage(path, this.pages) pages.set(path.absolutePath, page) } // ... }
284-295: Complex nested lookup could be simplified.The nested loop structure for finding pages in subPages could be more readable.
let page = this.pages.get(filepath) if (!page) { - let subPage: Page | undefined - for (const [_, pages] of this.subPages) { - subPage = pages.get(filepath) - if (subPage) { - break - } - } - page = subPage + page = Array.from(this.subPages.values()) + .map(pages => pages.get(filepath)) + .find(p => p !== undefined) }packages/core/src/page.ts (3)
65-69: Warn on mixed usage (definePage + ) to avoid silent precedenceRight now, macro wins silently if both are present. Consider emitting a warning to guide users.
Apply this localized change:
- const meta = await parseMacro(sfc) - if (meta) { - return meta - } + const meta = await parseMacro(sfc) + if (meta) { + if (sfc.customBlocks.some(b => b.type === 'route')) + this.ctx.logger?.warn(`Both definePage() and <route> block found. Using definePage() and ignoring <route> in ${this.path.relativePath}`, { timestamp: true }) + return meta + }Also applies to: 75-76
70-74: Script setup route-block restriction: confirm intended UX and documentReturning {} when scriptSetup exists (macro absent) forbids for setup components. If this is a deliberate constraint, surface a warning (once) to reduce confusion and document it in the README.
Proposed tweak:
- if (sfc.scriptSetup) { - // script setup 仅支持 macro 模式，不支持 route 自定义节点 - return {} - } + if (sfc.scriptSetup) { + debug.routeBlock(`script setup detected: only definePage() macro is supported for ${this.path.relativePath}`) + return {} + }Would you like me to add a brief docs blurb to the PR explaining this limitation and the migration path?
124-126: Nit: join imports with newlines for stable sourcemaps and readabilityIf you keep the imports-only approach anywhere, prefer newline-joined imports.
- const importScript = imports.join('') + const importScript = imports.join('\n')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
.vscode/launch.json(1 hunks)packages/core/build.config.ts(1 hunks)packages/core/client.d.ts(1 hunks)packages/core/index.d.ts(1 hunks)packages/core/package.json(3 hunks)packages/core/src/context.ts(7 hunks)packages/core/src/customBlock.ts(0 hunks)packages/core/src/index.ts(3 hunks)packages/core/src/page.ts(1 hunks)packages/core/src/types.ts(3 hunks)packages/core/src/utils.ts(3 hunks)packages/playground/package.json(1 hunks)packages/playground/src/pages/define-page/async-function.vue(1 hunks)packages/playground/src/pages/define-page/function.vue(1 hunks)packages/playground/src/pages/define-page/nested-function.vue(1 hunks)packages/playground/src/pages/define-page/object.vue(1 hunks)packages/playground/src/pages/define-page/option-api.vue(1 hunks)packages/playground/src/pages/define-page/remove-console.vue(1 hunks)packages/playground/src/pages/define-page/utils.ts(1 hunks)packages/playground/src/pages/define-page/yaml.vue(1 hunks)packages/playground/src/uni-pages.d.ts(1 hunks)packages/playground/tsconfig.json(1 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/customBlock.ts
🧰 Additional context used
🧬 Code graph analysis (11)
packages/playground/src/pages/define-page/nested-function.vue (2)
test/generate.spec.ts (1)
it(24-271)test/parser.spec.ts (2)
path(16-36)path(62-82)
packages/playground/tsconfig.json (4)
packages/playground/vite.config.ts (1)
UserConfig(7-9)packages/core/src/config/index.ts (1)
defineUniPages(5-7)packages/core/src/declaration.ts (2)
v(12-14)getDeclaration(8-40)packages/core/src/config/types/index.ts (2)
PagesConfig(23-145)UserPagesConfig(147-147)
packages/playground/src/pages/define-page/object.vue (2)
packages/core/src/config/types/index.ts (1)
PagesConfig(23-145)packages/playground/vite.config.ts (1)
UserConfig(7-9)
packages/playground/src/pages/define-page/async-function.vue (2)
test/generate.spec.ts (3)
ctx(242-270)ctx(106-198)ctx(25-104)test/parser.spec.ts (3)
path(16-36)path(38-60)path(62-82)
packages/playground/src/pages/define-page/yaml.vue (3)
test/parser.spec.ts (3)
path(62-82)path(16-36)path(38-60)packages/volar/src/index.ts (1)
provideCodeActions(36-44)packages/volar/src/yamlLs.ts (2)
createYamlLs(7-44)noop(5-5)
packages/playground/src/uni-pages.d.ts (1)
packages/core/src/declaration.ts (1)
getDeclaration(8-40)
packages/core/src/page.ts (4)
packages/core/src/context.ts (1)
PageContext(29-377)packages/core/src/types.ts (3)
PagePath(120-123)PageMetaDatum(125-140)UserPageMeta(147-153)packages/core/src/files.ts (1)
readFileSync(34-41)packages/core/src/utils.ts (4)
parseSFC(68-85)debug(27-37)babelGenerate(138-138)execScript(92-132)
packages/core/src/types.ts (1)
packages/core/client.d.ts (1)
definePage(10-10)
packages/core/src/context.ts (3)
packages/core/src/page.ts (1)
Page(13-105)packages/core/src/utils.ts (1)
debug(27-37)packages/core/src/types.ts (2)
PagePath(120-123)PageMetaDatum(125-140)
packages/core/src/index.ts (4)
packages/core/src/constant.ts (1)
FILE_EXTENSIONS(6-6)packages/core/src/utils.ts (1)
parseSFC(68-85)packages/core/src/page.ts (1)
findMacro(134-163)packages/core/src/customBlock.ts (1)
getRouteSfcBlock(88-95)
packages/core/src/utils.ts (1)
packages/core/src/customBlock.ts (1)
parseSFC(12-27)
🪛 ESLint
packages/playground/src/pages/define-page/utils.ts
[error] 2-2: Unexpected separator (;).
(style/member-delimiter-style)
[error] 3-3: Unexpected separator (;).
(style/member-delimiter-style)
packages/core/src/utils.ts
[error] 6-6: 'isCallOf' is defined but never used.
(unused-imports/no-unused-imports)
[error] 12-12: 't' is defined but never used.
(unused-imports/no-unused-imports)
🔇 Additional comments (21)
packages/playground/package.json (1)
17-18: Add YAML dependency for playground usage — LGTMThis aligns with the new define-page YAML demo. No issues from a runtime/API standpoint.
packages/playground/tsconfig.json (1)
10-10: Expose plugin typings to the Playground — LGTMAdding
@uni-helper/vite-plugin-uni-pagestocompilerOptions.typesmakes the globaldefinePagetypings available in the playground.packages/core/src/types.ts (2)
16-16: Good: centralized union for route-block languagesConsolidating the parser languages into a named alias improves reuse and consistency across the codebase.
74-74: AlignOptions.routeBlockLangto the new aliasSwitching to
RouteBlockLangkeeps the public surface DRY and makes future additions (e.g., toml) a single-point change.packages/playground/src/pages/define-page/object.vue (1)
3-5: LGTM: minimal page style via definePageUsing
style.navigationBarTitleTextthrough the macro matches the intended metadata flow.packages/playground/src/pages/define-page/nested-function.vue (1)
2-18: LGTM: clear, pure builder; easy to statically analyzeThe function-scoped title helper is simple and deterministic; a good showcase for the macro.
packages/playground/src/uni-pages.d.ts (1)
16-22: LGTM! New routes align with the definePage macro examples.The added routes correctly correspond to the new sample pages under the playground directory demonstrating various definePage usage patterns. The routes are properly typed for the NavigateToOptions interface.
packages/core/package.json (1)
60-75: No compatibility issues detected for Babel and Vue dependenciesI’ve checked the release notes and issue trackers for @babel/generator 7.28.3, @babel/types 7.28.2, and @vue/compiler-sfc 3.5.13—there are no widely reported or documented incompatibilities between these versions. To guard against potential Babel-version conflicts in your project, you may want to run
npm dedupe(or the Yarn equivalent) or otherwise ensure a single Babel version is installed in your lockfile. Everything else looks good to go!packages/core/src/index.ts (1)
108-109: Good enforcement of mutual exclusivity!The check properly prevents mixing
definePage()macro with<route/>blocks, which helps maintain consistency in the codebase.packages/core/src/context.ts (2)
35-36: Good architectural improvement with Map-based storage!Switching from arrays to Maps with absolute paths as keys improves lookup performance from O(n) to O(1), which is beneficial for large projects with many pages.
201-206: Efficient async page metadata fetching.Good use of Promise.all for parallel processing of page metadata, which improves performance compared to sequential processing.
packages/core/src/utils.ts (2)
134-136: Utility function correctly handles default exports.The
getDefaultExporthelper properly handles both ES6 default exports and CommonJS exports.
92-132: Execution ofexecScriptIs Limited to Build-Time Developer CodeThe
execScriptfunction is only invoked inpackages/core/src/page.tsto run SFC (Single-File Component) script blocks during static page generation, i.e. developer-authored code at build time—not arbitrary user-supplied snippets. Therefore, the critical security warning about untrusted input does not apply in this context.• Usage location:
packages/core/src/utils.ts– definition ofexecScript(lines 92–132)packages/core/src/page.ts– single call at line 128 to process.vueSFC scriptsGiven this, the proposed input‐validation guard and stricter TypeScript compilation flags are unnecessary here. No further changes are required.
Likely an incorrect or invalid review comment.
packages/core/src/page.ts (8)
41-47: Deriving page.path from file path looks goodNice defaulting strategy: stripping the extension and normalizing path ensures stable, platform-agnostic routes.
48-55: Change-detection cache is correct and side-effect freeStringifying the normalized options for a cheap equality check is reasonable here and aligns with watcher semantics.
81-96: Route block parsing and default type look goodJSON5/JSONC/YAML support with graceful failure and a sensible default for options.type = 'page' is pragmatic.
Also applies to: 97-101
114-116: Parsing config looks reasonableEnabling importAttributes supports JSON imports and aligns with user-land code patterns.
56-59: Error message includes relative path: good DXClearer stack context will help users pinpoint malformed route blocks quickly.
165-169: Utility to list imports is fine as a helperEven if not used post-refactor, keeping it exported is harmless; we can prune later if dead.
107-132: No additionaldefinePagemacro shapes detected
We ran the repository-wide search for the following patterns and only found the existing inline object-literal calls in the playground examples—noexport default definePage(…)or identifier-based invocations:
- export default definePage(…): 0 matches
- definePage(variable): 0 matches
- definePage({…}): 2 matches (packages/playground/src/pages/define-page/*.vue)
Since
parseMacroalready handles the inline object-literal form used across the repo, no changes are required at this time.
62-62: Fix invalid encoding literal passed to fs.readFileSyncUsing "utf-8" is not a valid BufferEncoding in Node's typings and will cause a TS compile error. Use "utf8" (no hyphen), or switch to the repo’s resilient read helper for consistency.
Apply the minimal fix:
- const content = readFileSync(this.path.absolutePath, 'utf-8') + const content = readFileSync(this.path.absolutePath, 'utf8')Optional (for consistency with packages/core/src/files.ts and to avoid throwing on missing files):
- import { readFileSync } from 'node:fs' + import { readFileSync } from './files' ... - const content = readFileSync(this.path.absolutePath, 'utf8') + const content = readFileSync(this.path.absolutePath)Likely an incorrect or invalid review comment.
| "type": "node", | ||
| "request": "launch", | ||
| "cwd": "${workspaceFolder}/packages/playground", | ||
| "preLaunchTask": "npm: debug - packages/core", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure the preLaunch task uses pnpm, not npm
"preLaunchTask": "npm: debug - packages/core" relies on VS Code’s package manager setting. If npm.packageManager isn’t set to pnpm, this will run via npm and may break a pnpm workspace.
Option A: add a workspace setting .vscode/settings.json:
{
"npm.packageManager": "pnpm"
}Option B: replace the preLaunch task with a shell task that invokes pnpm explicitly.
🏁 Script executed:
#!/bin/bash
# Verify VS Code is configured to use pnpm for npm tasks
if [[ -f .vscode/settings.json ]]; then
echo "Detected .vscode/settings.json:"
cat .vscode/settings.json
echo
echo "Configured package manager:"
rg -n '"npm.packageManager"\s*:\s*"pnpm"' .vscode/settings.json || {
echo "Missing npm.packageManager=pnpm setting"; exit 1;
}
else
echo "No .vscode/settings.json found. Recommend setting npm.packageManager to pnpm."; exit 2;
fiLength of output: 1526
Require pnpm for VS Code preLaunchTask
Your workspace has a .vscode/settings.json but it’s missing the npm.packageManager setting, so VS Code will default to using npm for "npm: debug - packages/core", which breaks pnpm workspaces.
To resolve:
- In
.vscode/settings.json, add the pnpm package manager setting:{ // …existing settings… "npm.packageManager": "pnpm" } - Or, explicitly invoke pnpm in your launch configuration (
.vscode/launch.json, around line 12):- "preLaunchTask": "npm: debug - packages/core", + "preLaunchTask": "pnpm: debug - packages/core",
Either approach will ensure VS Code runs your debug task with pnpm rather than npm.
🤖 Prompt for AI Agents
.vscode/launch.json around line 12: the preLaunchTask "npm: debug -
packages/core" will cause VS Code to use npm by default which breaks pnpm
workspaces; either add "npm.packageManager": "pnpm" to .vscode/settings.json so
VS Code uses pnpm for npm scripts, or change the launch.json preLaunchTask to
explicitly call the pnpm task (replace the npm task reference with the
equivalent pnpm task name or invoke a task that runs "pnpm run debug
--workspace=packages/core"). Ensure the chosen change is committed so VS Code
runs the debug task with pnpm.
| export * from './dist' | ||
|
|
||
| export { default } from './dist' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm default export exists in ./dist, or drop the default re-export
Re-exporting default from './dist' will fail type resolution if the dist entry has no default export. Keep the named export star and only keep the default re-export if the compiled output exposes a default.
Run this quick check:
If no default is present, adjust this file:
-export { default } from './dist'
+// export { default } from './dist' // keep only if './dist' provides a default export🏁 Script executed:
#!/bin/bash
# Verify a default export exists in dist or source
rg -nP 'export\s+default\b' packages/core/dist 2>/dev/null || rg -nP 'export\s+default\b' packages/core/src || echo "No default export detected"Length of output: 230
Drop the default re-export in packages/core/index.d.ts
The packages/core/dist entrypoint does not actually export a default, so the line
export { default } from './dist'will break downstream type resolution. Remove it (or add a default export in your build if you intended one).
• Location: packages/core/index.d.ts
• Change:
-export { default } from './dist'
+// export { default } from './dist' // removed—no default export in dist🤖 Prompt for AI Agents
In packages/core/index.d.ts around lines 3 to 5, the file re-exports a
non-existent default from './dist' which breaks downstream type resolution;
remove the line "export { default } from './dist'" (or alternatively add a real
default export in the built ./dist types if you intended one) so that only the
named export re-exports remain and TypeScript consumers won't receive an
incorrect default export.
| ".": { | ||
| "import": { | ||
| "types": "./dist/index.d.mts", | ||
| "types": "./index.d.ts", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider the impact of changing type declaration exports.
Moving type declarations from ./dist/index.d.ts to ./index.d.ts changes the public API surface. This could break consumers who directly import from the dist folder.
Let me verify if there's a corresponding index.d.ts file at the root and check for any direct imports in the codebase:
Also applies to: 34-34, 44-44, 48-48
🏁 Script executed:
#!/bin/bash
# Check if index.d.ts exists at the package root
fd -t f "^index\.d\.ts$" packages/core --max-depth 1
# Search for any direct imports from dist folder in TypeScript files
rg -n --type=ts "from\s+['\"]@uni-helper/vite-plugin-uni-pages/dist" -g '!node_modules/**' -g '!dist/**'
# Check if there's a build step that generates index.d.ts at root
rg -n "index\.d\.ts" packages/core/build.config.tsLength of output: 241
Ensure deep‐imports of type declarations remain compatible
I confirmed that packages/core/index.d.ts exists at the package root, but our build setup does not emit or copy any .d.ts into packages/core/dist. This means consumers who import types via
import { Foo } from '@uni-helper/vite-plugin-uni-pages/dist'will no longer find dist/index.d.ts after this change.
Action items to avoid a breaking change:
- Add a
packages/core/dist/index.d.tsfile (for example, withexport * from '../index';) or update the build to copy the root declaration intodist. - Alternatively, use the
"exports"field inpackage.jsonto redirect deep imports back to the root types. - Update documentation to discourage direct imports from
dist, or bump the major version and call out this breaking change in the changelog.
packages/core/src/index.ts
Outdated
| const sfc = await parseSFC(code, { filename: id }) | ||
|
|
||
| let macro: CallExpression | undefined | ||
| if (sfc.scriptSetup) { | ||
| const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') | ||
| macro = findMacro(ast.body, sfc.filename) | ||
| } | ||
|
|
||
| if (!macro && sfc.script) { | ||
| const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') | ||
| macro = findMacro(ast.body, sfc.filename) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null checks for SFC blocks before accessing properties.
While the code checks for the existence of sfc.scriptSetup and sfc.script, it doesn't validate that these blocks have valid content before parsing.
if (sfc.scriptSetup) {
+ if (!sfc.scriptSetup.content) {
+ debug.definePage(`scriptSetup block has no content: ${sfc.filename}`)
+ } else {
const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js')
macro = findMacro(ast.body, sfc.filename)
+ }
}
if (!macro && sfc.script) {
+ if (!sfc.script.content) {
+ debug.definePage(`script block has no content: ${sfc.filename}`)
+ } else {
const ast = babelParse(sfc.script.content, sfc.script.lang || 'js')
macro = findMacro(ast.body, sfc.filename)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sfc = await parseSFC(code, { filename: id }) | |
| let macro: CallExpression | undefined | |
| if (sfc.scriptSetup) { | |
| const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') | |
| macro = findMacro(ast.body, sfc.filename) | |
| } | |
| if (!macro && sfc.script) { | |
| const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') | |
| macro = findMacro(ast.body, sfc.filename) | |
| } | |
| const sfc = await parseSFC(code, { filename: id }) | |
| let macro: CallExpression | undefined | |
| if (sfc.scriptSetup) { | |
| if (!sfc.scriptSetup.content) { | |
| debug.definePage(`scriptSetup block has no content: ${sfc.filename}`) | |
| } else { | |
| const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') | |
| macro = findMacro(ast.body, sfc.filename) | |
| } | |
| } | |
| if (!macro && sfc.script) { | |
| if (!sfc.script.content) { | |
| debug.definePage(`script block has no content: ${sfc.filename}`) | |
| } else { | |
| const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') | |
| macro = findMacro(ast.body, sfc.filename) | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/index.ts around lines 90 to 101, the code assumes
sfc.scriptSetup and sfc.script contain valid content and lang before parsing;
add null/undefined and empty-string checks for those SFC blocks (e.g., verify
sfc.scriptSetup && typeof sfc.scriptSetup.content === 'string' &&
sfc.scriptSetup.content.trim() !== '' before calling babelParse, and similarly
for sfc.script) so you only call babelParse on valid text (and default lang only
when lang is present), and optionally wrap parse calls in try/catch to handle
malformed content gracefully.
packages/core/src/page.ts
Outdated
| export async function parseMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { | ||
| const sfcScript = sfc.scriptSetup || sfc.script | ||
|
|
||
| if (!sfcScript) { | ||
| return undefined | ||
| } | ||
|
|
||
| const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { | ||
| plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], | ||
| }) | ||
| const macro = findMacro(ast.body, sfc.filename) | ||
| if (macro) { | ||
| const imports = findImports(ast.body).map(imp => babelGenerate(imp).code) | ||
|
|
||
| const [macroOption] = macro.arguments | ||
| const code = babelGenerate(macroOption).code | ||
| let script = '' | ||
| const importScript = imports.join('') | ||
| script += importScript | ||
| script += `export default ${code}` | ||
|
|
||
| const result = await execScript(script, sfc.filename) | ||
| return result as UserPageMeta | ||
| } | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
definePage macro execution can break when the argument references local bindings; export only imports is insufficient
Current approach emits only imports + the macro argument, dropping other top-level declarations (consts, functions) the macro may reference, causing ReferenceError at runtime. It also doesn’t guard against missing arguments.
Apply this refactor to preserve all top-level code (except the macro call itself and any existing default export), then export the macro argument as default for evaluation:
export async function parseMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> {
const sfcScript = sfc.scriptSetup || sfc.script
if (!sfcScript) {
return undefined
}
const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', {
plugins: [['importAttributes', { deprecatedAssertSyntax: true }]],
})
const macro = findMacro(ast.body, sfc.filename)
if (macro) {
- const imports = findImports(ast.body).map(imp => babelGenerate(imp).code)
-
- const [macroOption] = macro.arguments
- const code = babelGenerate(macroOption).code
- let script = ''
- const importScript = imports.join('')
- script += importScript
- script += `export default ${code}`
-
- const result = await execScript(script, sfc.filename)
- return result as UserPageMeta
+ if (macro.arguments.length === 0)
+ throw new Error(`definePage() requires one argument: ${sfc.filename}`)
+
+ const [macroOption] = macro.arguments
+ // Build a new program:
+ // - keep all top-level statements except:
+ // a) the definePage(...) call statement itself
+ // b) any existing export default (to avoid duplicate default exports)
+ const preserved = ast.body.filter(stmt => {
+ if (t.isExpressionStatement(stmt) && isCallOf((stmt as any).expression, 'definePage'))
+ return false
+ if (t.isExportDefaultDeclaration(stmt))
+ return false
+ return true
+ })
+ const program = t.program(preserved, [], 'module')
+ program.body.push(t.exportDefaultDeclaration(macroOption as t.Expression))
+
+ const script = babelGenerate(program).code
+ const result = await execScript(script, sfc.filename)
+ return result as UserPageMeta
}
return undefined
}Notes:
- This preserves local constants/functions so expressions like definePage({ title }) work.
- It avoids duplicate default exports when <script> already has export default defineComponent(...).
- We still support <script setup> because there’s no default export there.
If you prefer keeping imports-only, we can instead statically collect and emit all top-level non-export declarations (VariableDeclaration/FunctionDeclaration/ClassDeclaration) alongside imports, but the above is simpler and more robust.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function parseMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { | |
| const sfcScript = sfc.scriptSetup || sfc.script | |
| if (!sfcScript) { | |
| return undefined | |
| } | |
| const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { | |
| plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], | |
| }) | |
| const macro = findMacro(ast.body, sfc.filename) | |
| if (macro) { | |
| const imports = findImports(ast.body).map(imp => babelGenerate(imp).code) | |
| const [macroOption] = macro.arguments | |
| const code = babelGenerate(macroOption).code | |
| let script = '' | |
| const importScript = imports.join('') | |
| script += importScript | |
| script += `export default ${code}` | |
| const result = await execScript(script, sfc.filename) | |
| return result as UserPageMeta | |
| } | |
| return undefined | |
| } | |
| export async function parseMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { | |
| const sfcScript = sfc.scriptSetup || sfc.script | |
| if (!sfcScript) { | |
| return undefined | |
| } | |
| const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { | |
| plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], | |
| }) | |
| const macro = findMacro(ast.body, sfc.filename) | |
| if (macro) { | |
| if (macro.arguments.length === 0) | |
| throw new Error(`definePage() requires one argument: ${sfc.filename}`) | |
| const [macroOption] = macro.arguments | |
| // Build a new program: | |
| // - keep all top-level statements except: | |
| // a) the definePage(...) call statement itself | |
| // b) any existing export default (to avoid duplicate default exports) | |
| const preserved = ast.body.filter(stmt => { | |
| if (t.isExpressionStatement(stmt) && isCallOf((stmt as any).expression, 'definePage')) | |
| return false | |
| if (t.isExportDefaultDeclaration(stmt)) | |
| return false | |
| return true | |
| }) | |
| const program = t.program(preserved, [], 'module') | |
| program.body.push(t.exportDefaultDeclaration(macroOption as t.Expression)) | |
| const script = babelGenerate(program).code | |
| const result = await execScript(script, sfc.filename) | |
| return result as UserPageMeta | |
| } | |
| return undefined | |
| } |
| tabbar: {}, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
tabbar is not a valid page-level key; remove or move to root tabBar
tabBar belongs to the app-level pages config, not individual page meta. Also the key here is lowercased (tabbar) which won’t match the schema anyway.
Apply:
- tabbar: {},If you intended to tweak the global tab bar in the playground, move it into the root pages.json (or the equivalent config source) under tabBar.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| tabbar: {}, | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In packages/playground/src/pages/define-page/object.vue around lines 9 to 10,
the object includes a lowercased tabbar property which is not a valid page-level
key; remove this property from the page meta and, if you intended to configure
the app tab bar, move or add the configuration to the root pages.json (or your
global pages config) under the correct camelCase key tabBar so it applies at the
app level.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/src/customBlock.ts (1)
8-8: Remove unused type import ResolvedOptions.ESLint reports this correctly. Trim the import to avoid noisy diffs and keep the bundle clean.
-import type { CustomBlock, ResolvedOptions, RouteBlockLang } from './types' +import type { CustomBlock, RouteBlockLang } from './types'
🧹 Nitpick comments (8)
test/parser.spec.ts (3)
20-23: Avoid unnecessary awaits; both getRouteSfcBlock and getRouteBlock are synchronous.These calls return values synchronously, so awaiting them adds noise and may mislead future readers into thinking they are async. Tighten the tests by removing await here.
- const sfc = parseSFC(readFileSync(path, 'utf-8'), { filename: path }) - const str = await getRouteSfcBlock(sfc) - const routeBlock = await getRouteBlock(path, str, options.routeBlockLang) + const sfc = parseSFC(readFileSync(path, 'utf-8'), { filename: path }) + const str = getRouteSfcBlock(sfc) + const routeBlock = getRouteBlock(path, str, options.routeBlockLang)Also applies to: 43-46, 68-71
17-39: Add a regression test for routeBlockLang fallback when no lang attribute is set.To ensure parseCustomBlock respects the configured default when block.lang is absent, add a minimal SFC fixture without lang on the block and assert that options.routeBlockLang is used.
I can draft the fixture and test if helpful.
41-63: Snapshot is clear and stable. Consider also asserting types where relevant.Inline snapshots look good. For stronger signals, assert routeBlock.attr.lang and attr.type explicitly alongside the snapshot to fail fast on signature regressions.
packages/core/src/customBlock.ts (2)
10-21: Default lang resolution is sensible; minor naming nit.const lang = block.lang ?? routeBlockLang is correct and matches the new test flow. Consider renaming filePath to path or sourcePath for consistency with getRouteBlock(path, ...).
23-62: Error messages are actionable; add filename to yaml/jsonc parse errors consistently.You already include filePath and block.type, which is great. Optionally add a short hint about supported languages in the thrown error to speed up troubleshooting when lang is misspelled.
packages/core/src/utils.ts (3)
81-87: Tighten the sandbox and dependency resolution.
- Use createRequire(filename) to resolve relative imports exactly like Node would from the file, not the directory.
- Optionally restrict require to a whitelist if untrusted code might run here (e.g., only allow local paths or known packages) to minimize supply-chain and escape risks.
71-76: Enable better dev ergonomics with source maps and fileName during transpile.Including fileName and inline source maps makes stack traces match the original TS lines, which is important when macro execution throws.
23-33: Nice addition: debug.definePage channel.Instrumenting definePage flow will help diagnose macro evaluation. Consider documenting the env var to enable it in the README.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/core/src/customBlock.ts(2 hunks)packages/core/src/index.ts(3 hunks)packages/core/src/page.ts(1 hunks)packages/core/src/utils.ts(3 hunks)test/parser.spec.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/index.ts
- packages/core/src/page.ts
🧰 Additional context used
🧬 Code graph analysis (3)
test/parser.spec.ts (2)
packages/core/src/page.ts (1)
parseSFC(82-99)packages/core/src/customBlock.ts (2)
getRouteSfcBlock(69-71)getRouteBlock(73-77)
packages/core/src/utils.ts (1)
packages/core/src/context.ts (4)
PageContext(32-365)virtualModule(344-348)path(133-145)v(313-313)
packages/core/src/customBlock.ts (1)
packages/core/src/types.ts (2)
RouteBlockLang(16-16)CustomBlock(7-10)
🪛 ESLint
packages/core/src/customBlock.ts
[error] 8-8: 'ResolvedOptions' is defined but never used.
(unused-imports/no-unused-imports)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (3)
test/parser.spec.ts (1)
12-16: Good shift to descriptor-based parsing.Switching tests to parse SFCs up front and passing the descriptor improves separation of concerns and mirrors core API changes. No issues found.
packages/core/src/customBlock.ts (1)
69-77: All call sites updated to new signatures – no further action requiredA repository-wide search found only two usage sites (tests in test/parser.spec.ts and the helper in packages/core/src/page.ts), and all calls now pass an SFCDescriptor to getRouteSfcBlock and a string, SFCBlock, and RouteBlockLang to getRouteBlock. No old-style invocations remain, so no shim overloads or call-site edits are needed.
packages/core/src/utils.ts (1)
111-116: babelGenerate alias looks good; keep getDefaultExport local.This neatens interop with @babel/generator’s default export across CJS/ESM. No changes needed.
| export async function execScript(code: string, filename: string): Promise<any> { | ||
| // 编译 TypeScript 代码为 JavaScript | ||
| const jsCode = ts.transpileModule(code, { | ||
| compilerOptions: { | ||
| module: ts.ModuleKind.CommonJS, | ||
| target: ts.ScriptTarget.ESNext, | ||
| }, | ||
| }).outputText | ||
|
|
||
| function parseData(block?: SFCBlock) { | ||
| return { | ||
| content: block?.loc.source.trim() ?? '', | ||
| attr: block?.attrs ?? '', | ||
| } | ||
| } | ||
| const dir = path.dirname(filename) | ||
|
|
||
| function setCache(filePath: string, routeBlock?: SFCBlock) { | ||
| pages.set(filePath, cjStringify(parseData(routeBlock))) | ||
| // 创建一个新的虚拟机上下文 | ||
| const vmContext = { | ||
| require: createRequire(dir), | ||
| module: {}, | ||
| exports: {}, | ||
| __filename: filename, | ||
| __dirname: dir, | ||
| } | ||
|
|
||
| async function hasChanged(filePath: string, routeBlock?: SFCBlock) { | ||
| if (!routeBlock) | ||
| routeBlock = await getRouteSfcBlock(normalizePath(filePath)) | ||
| // 使用 vm 模块执行 JavaScript 代码 | ||
| const script = new vm.Script(jsCode) | ||
|
|
||
| return !pages.has(filePath) || cjStringify(parseData(routeBlock)) !== pages.get(filePath) | ||
| try { | ||
| script.runInNewContext(vmContext) | ||
| } | ||
| catch (error: any) { | ||
| throw new Error(`${filename}: ${error.message}`) | ||
| } | ||
|
|
||
| return { | ||
| setCache, | ||
| hasChanged, | ||
| // 获取导出的值 | ||
| const result = (vmContext.exports as any).default || vmContext.exports | ||
|
|
||
| // 如果是函数，执行函数并返回其返回值 | ||
| if (typeof result === 'function') { | ||
| return Promise.resolve(result()) | ||
| } | ||
|
|
||
| // 如果不是函数，返回结果 | ||
| return Promise.resolve(result) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
execScript does not correctly mirror CommonJS module semantics; module.exports changes may be lost.
You initialize module as {} and exports as {} independently and then read only from vmContext.exports. If a script assigns module.exports = fn/object, you’ll miss it. Also, require should resolve relative to the file, not the directory.
Apply the following refactor to align with Node CJS semantics, improve debugging, and keep support for both exports.default and module.exports:
-export async function execScript(code: string, filename: string): Promise<any> {
- // 编译 TypeScript 代码为 JavaScript
- const jsCode = ts.transpileModule(code, {
- compilerOptions: {
- module: ts.ModuleKind.CommonJS,
- target: ts.ScriptTarget.ESNext,
- },
- }).outputText
-
- const dir = path.dirname(filename)
-
- // 创建一个新的虚拟机上下文
- const vmContext = {
- require: createRequire(dir),
- module: {},
- exports: {},
- __filename: filename,
- __dirname: dir,
- }
-
- // 使用 vm 模块执行 JavaScript 代码
- const script = new vm.Script(jsCode)
-
- try {
- script.runInNewContext(vmContext)
- }
- catch (error: any) {
- throw new Error(`${filename}: ${error.message}`)
- }
-
- // 获取导出的值
- const result = (vmContext.exports as any).default || vmContext.exports
-
- // 如果是函数，执行函数并返回其返回值
- if (typeof result === 'function') {
- return Promise.resolve(result())
- }
-
- // 如果不是函数，返回结果
- return Promise.resolve(result)
-}
+export async function execScript(code: string, filename: string): Promise<any> {
+ const transpiled = ts.transpileModule(code, {
+ compilerOptions: {
+ module: ts.ModuleKind.CommonJS,
+ target: ts.ScriptTarget.ESNext,
+ sourceMap: true,
+ inlineSourceMap: true,
+ },
+ fileName: filename,
+ })
+ // Help stack traces map to the source file in dev
+ const jsCode = `${transpiled.outputText}\n//# sourceURL=${filename}`
+ const dir = path.dirname(filename)
+
+ // Create a Node-like CJS environment
+ const module = { exports: {} as any }
+ const exports = module.exports
+ const require = createRequire(filename)
+
+ const context = vm.createContext({
+ require,
+ module,
+ exports,
+ __filename: filename,
+ __dirname: dir,
+ console, // preserve developer ergonomics
+ })
+
+ const script = new vm.Script(jsCode, { filename })
+ try {
+ // Consider making timeout configurable if long-running user code is expected
+ script.runInContext(context, { timeout: 1000, displayErrors: true })
+ }
+ catch (error: any) {
+ throw new Error(`${filename}: ${error.message}`)
+ }
+
+ // Prefer module.exports, then exports, then default on either
+ const ex = (context.module as any)?.exports ?? (context as any).exports
+ const result = (ex && ex.__esModule && ex.default) ? ex.default : (ex?.default ?? ex)
+ return typeof result === 'function' ? await result() : result
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function execScript(code: string, filename: string): Promise<any> { | |
| // 编译 TypeScript 代码为 JavaScript | |
| const jsCode = ts.transpileModule(code, { | |
| compilerOptions: { | |
| module: ts.ModuleKind.CommonJS, | |
| target: ts.ScriptTarget.ESNext, | |
| }, | |
| }).outputText | |
| function parseData(block?: SFCBlock) { | |
| return { | |
| content: block?.loc.source.trim() ?? '', | |
| attr: block?.attrs ?? '', | |
| } | |
| } | |
| const dir = path.dirname(filename) | |
| function setCache(filePath: string, routeBlock?: SFCBlock) { | |
| pages.set(filePath, cjStringify(parseData(routeBlock))) | |
| // 创建一个新的虚拟机上下文 | |
| const vmContext = { | |
| require: createRequire(dir), | |
| module: {}, | |
| exports: {}, | |
| __filename: filename, | |
| __dirname: dir, | |
| } | |
| async function hasChanged(filePath: string, routeBlock?: SFCBlock) { | |
| if (!routeBlock) | |
| routeBlock = await getRouteSfcBlock(normalizePath(filePath)) | |
| // 使用 vm 模块执行 JavaScript 代码 | |
| const script = new vm.Script(jsCode) | |
| return !pages.has(filePath) || cjStringify(parseData(routeBlock)) !== pages.get(filePath) | |
| try { | |
| script.runInNewContext(vmContext) | |
| } | |
| catch (error: any) { | |
| throw new Error(`${filename}: ${error.message}`) | |
| } | |
| return { | |
| setCache, | |
| hasChanged, | |
| // 获取导出的值 | |
| const result = (vmContext.exports as any).default || vmContext.exports | |
| // 如果是函数，执行函数并返回其返回值 | |
| if (typeof result === 'function') { | |
| return Promise.resolve(result()) | |
| } | |
| // 如果不是函数，返回结果 | |
| return Promise.resolve(result) | |
| } | |
| export async function execScript(code: string, filename: string): Promise<any> { | |
| const transpiled = ts.transpileModule(code, { | |
| compilerOptions: { | |
| module: ts.ModuleKind.CommonJS, | |
| target: ts.ScriptTarget.ESNext, | |
| sourceMap: true, | |
| inlineSourceMap: true, | |
| }, | |
| fileName: filename, | |
| }) | |
| // Help stack traces map to the source file in dev | |
| const jsCode = `${transpiled.outputText}\n//# sourceURL=${filename}` | |
| const dir = path.dirname(filename) | |
| // Create a Node-like CJS environment | |
| const module = { exports: {} as any } | |
| const exports = module.exports | |
| const require = createRequire(filename) | |
| const context = vm.createContext({ | |
| require, | |
| module, | |
| exports, | |
| __filename: filename, | |
| __dirname: dir, | |
| console, // preserve developer ergonomics | |
| }) | |
| const script = new vm.Script(jsCode, { filename }) | |
| try { | |
| // Consider making timeout configurable if long-running user code is expected | |
| script.runInContext(context, { timeout: 1000, displayErrors: true }) | |
| } catch (error: any) { | |
| throw new Error(`${filename}: ${error.message}`) | |
| } | |
| // Prefer module.exports, then exports, then default on either | |
| const ex = (context.module as any)?.exports ?? (context as any).exports | |
| const result = (ex && ex.__esModule && ex.default) | |
| ? ex.default | |
| : (ex?.default ?? ex) | |
| return typeof result === 'function' ? await result() : result | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/utils.ts around lines 69–109, the VM context currently
creates independent module and exports objects and uses createRequire(dir),
which breaks CommonJS semantics and relative resolution; update the context to
mirror Node CJS by creating module = { exports: {} } and setting exports =
module.exports (so assignments to module.exports are preserved), call
createRequire(filename) so require resolves relative to the file path, run the
script in that context, and on error include the original error stack/message
for better debugging; after execution prefer module.exports, then
module.exports.default, then exports as the result, and if the final export is a
function support both sync and async invocation (await or return its resolved
value).
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
packages/core/src/index.ts (1)
94-102: Add null/empty-content checks before parsing script blocks (dup of prior feedback)Avoid parsing empty script blocks; reduces noise and parse errors.
- if (sfc.scriptSetup) { + if (sfc.scriptSetup && typeof sfc.scriptSetup.content === 'string' && sfc.scriptSetup.content.trim()) { const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') macro = findMacro(ast.body, sfc.filename) } - if (!macro && sfc.script) { + if (!macro && sfc.script && typeof sfc.script.content === 'string' && sfc.script.content.trim()) { const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') macro = findMacro(ast.body, sfc.filename) }packages/core/src/page.ts (1)
137-166: Broaden macro detection and improve errors (dup of prior feedback)Detect
export default definePage(...), allow identifier/object/function args, and include filename in duplicate-call errors. This unblocks common patterns.export function findMacro(stmts: t.Statement[], filename: string): t.CallExpression | undefined { - const nodes = stmts - .map((raw: t.Node) => { - let node = raw - if (raw.type === 'ExpressionStatement') - node = raw.expression - return isCallOf(node, 'definePage') ? node : undefined - }) - .filter((node): node is t.CallExpression => !!node) + const nodes = stmts + .map((raw: t.Node) => { + let node: t.Node | undefined = raw + if (t.isExpressionStatement(raw)) + node = raw.expression + else if (t.isExportDefaultDeclaration(raw)) + node = raw.declaration as t.Node + return node && isCallOf(node as any, 'definePage') ? (node as t.CallExpression) : undefined + }) + .filter((node): node is t.CallExpression => !!node) if (!nodes.length) return - if (nodes.length > 1) - throw new Error(`duplicate definePage() call`) + if (nodes.length > 1) + throw new Error(`duplicate definePage() call in ${filename}`) - // 仅第支持一个 definePage + // 仅支持一个 definePage const macro = nodes[0] - // 提取 macro function 内的第一个参数 + // 提取第一个参数 const [opt] = macro.arguments - // 检查 macro 的参数是否正确 - if (opt && !t.isFunctionExpression(opt) && !t.isArrowFunctionExpression(opt) && !t.isObjectExpression(opt)) { - debug.definePage(`definePage() only accept argument in function or object: ${filename}`) - return - } + if (!opt) { + throw new Error(`definePage() missing argument in ${filename}`) + } + if (!t.isExpression(opt)) { + throw new Error(`definePage() argument must be an expression in ${filename}`) + } return macro }
🧹 Nitpick comments (3)
test/generate.spec.ts (1)
101-165: Stabilize tests against UNI_PLATFORM env leakageSnapshots assume no platform suffix. If someone runs tests with
UNI_PLATFORMset, expectations may drift. Clear it around these tests to make them hermetic.You can add this near the top of the file:
import { beforeEach, afterAll } from 'vitest' const __OLD_UNI_PLATFORM__ = process.env.UNI_PLATFORM beforeEach(() => { delete process.env.UNI_PLATFORM }) afterAll(() => { process.env.UNI_PLATFORM = __OLD_UNI_PLATFORM__ })Also applies to: 259-323
packages/core/src/index.ts (2)
87-91: Guard against query suffixes and non-page files when gating by extension
id.endsWith(ext)can fail with?query (e.g.,Foo.vue?vue&type=script). Strip the query and match.<ext>.- if (!FILE_EXTENSIONS.find(ext => id.endsWith(ext))) { - return null - } + const fileId = id.split('?', 1)[0] + if (!FILE_EXTENSIONS.some(ext => fileId.endsWith(`.${ext}`))) + return null
116-125: Route block removal by regex is brittle; prefer descriptor offsetsRegex can misfire on strings/comments. Use
block.loc.start.offset/end.offsetfor each<route>custom block.- if (routeBlock) { - const routeBlockMatches = s.original.matchAll( - /<route[^>]*>([\s\S]*?)<\/route>/g, - ) - for (const match of routeBlockMatches) { - const index = match.index! - const length = match[0].length - s.remove(index, index + length) - } - } + if (routeBlock && sfc.customBlocks?.length) { + for (const b of sfc.customBlocks.filter(b => b.type === 'route')) { + const start = b.loc?.start?.offset + const end = b.loc?.end?.offset + if (start != null && end != null) + s.remove(start, end) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
package.json(1 hunks)packages/core/src/index.ts(3 hunks)packages/core/src/page.ts(1 hunks)packages/core/src/uni-platform.ts(1 hunks)packages/playground/src/pages/define-page/async-function.vue(1 hunks)packages/playground/src/pages/define-page/conditional-compilation.vue(1 hunks)test/files.spec.ts(1 hunks)test/generate.spec.ts(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- test/files.spec.ts
- package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/playground/src/pages/define-page/async-function.vue
🧰 Additional context used
🧬 Code graph analysis (4)
packages/playground/src/pages/define-page/conditional-compilation.vue (1)
packages/core/src/context.ts (1)
PageContext(32-365)
test/generate.spec.ts (1)
test/parser.spec.ts (4)
path(62-82)path(16-36)path(38-60)it(15-83)
packages/core/src/index.ts (2)
packages/core/src/constant.ts (1)
FILE_EXTENSIONS(6-6)packages/core/src/page.ts (2)
parseSFC(75-92)findMacro(137-166)
packages/core/src/page.ts (5)
packages/core/src/context.ts (6)
PageContext(29-377)PageContext(32-365)parsePage(180-195)mergePageMetaData(247-252)parsePages(204-217)page(205-205)packages/core/src/types.ts (4)
PagePath(120-123)PageMetaDatum(125-140)UserPageMeta(147-153)RouteBlockLang(16-16)packages/core/src/files.ts (1)
readFileSync(34-41)packages/core/src/utils.ts (3)
babelGenerate(115-115)execScript(69-109)debug(23-33)packages/core/src/customBlock.ts (2)
getRouteSfcBlock(69-71)getRouteBlock(73-77)
🪛 GitHub Actions: CI
packages/playground/src/pages/define-page/conditional-compilation.vue
[error] 1-1: test/generate.spec.ts > generate routes > vue - pages snapshot: Read page options fail in conditional-compilation.vue: ES Module ast-kit/dist/index.js cannot be required by CommonJS; use dynamic import() as recommended.
test/generate.spec.ts
[error] 1-1: Vitest test run failed in 'generate routes': Read page options fail in conditional-compilation.vue (ES Module import issue).
packages/core/src/page.ts
[error] 58-58: Read page options fail in conditional-compilation.vue during Page.readPageMeta: ES Module import incompatibility detected (require() of ast-kit from core/dist/index.cjs).
🪛 GitHub Check: test (windows-latest, 18)
packages/core/src/page.ts
[failure] 58-58: test/generate.spec.ts > generate routes > vue - not merge pages snapshot
Error: Read page options fail in ..\packages\playground\src\pages\define-page\conditional-compilation.vue
D:/a/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\node_modules.pnpm\[email protected]\node_modules\ast-kit\dist\index.js from D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs not supported.
Instead change the require of index.js in D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:174:5
[failure] 58-58: test/generate.spec.ts > generate routes > vue - pages snapshot
Error: Read page options fail in ..\packages\playground\src\pages\define-page\conditional-compilation.vue
D:/a/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\node_modules.pnpm\[email protected]\node_modules\ast-kit\dist\index.js from D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs not supported.
Instead change the require of index.js in D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:28:5
🪛 GitHub Check: test (ubuntu-latest, 18)
packages/core/src/page.ts
[failure] 58-58: test/generate.spec.ts > generate routes > vue - not merge pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:174:5
[failure] 58-58: test/generate.spec.ts > generate routes > vue - pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:28:5
🪛 GitHub Check: test (macos-latest, 18)
packages/core/src/page.ts
[failure] 58-58: test/generate.spec.ts > generate routes > vue - not merge pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:174:5
[failure] 58-58: test/generate.spec.ts > generate routes > vue - pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:28:5
🔇 Additional comments (5)
packages/core/src/uni-platform.ts (1)
3-21: Enum looks complete; keep as single source of truthEnum values align with UniApp platform identifiers. No issues spotted.
packages/playground/src/pages/define-page/conditional-compilation.vue (2)
2-2: Heads-up: macro execution importing the plugin triggers ESM-in-CJS failure in testsThis page’s macro imports
@uni-helper/vite-plugin-uni-pages; when we evaluate the macro in a CJS VM during tests, that import resolves tocore/dist/index.cjswhichrequires ESM-onlyast-kit, causing the CI error. I’ll propose a core-side fix to stub these imports during macro evaluation, so no change is required here.After applying the core fix I’m proposing in
packages/core/src/page.ts(see below), please re-run CI to confirm the snapshots pass.Also applies to: 10-12
5-22: Page meta generation via definePage is fineTitle derivation and middleware array are straightforward; no functional concerns.
test/generate.spec.ts (1)
31-167: Snapshots align with new definePage pagesThe added routes and meta look consistent with the playground pages. Once the macro-eval fix lands, these should pass unchanged.
packages/core/src/page.ts (1)
121-135: Route-block fallback logic is goodDefaults to
{ type: 'page' }when no block found; merging viacjAssignpreserves comments correctly.
| const s = new MagicString(code) | ||
| const routeBlockMatches = s.original.matchAll( | ||
| /<route[^>]*>([\s\S]*?)<\/route>/g, | ||
| ) | ||
|
|
||
| for (const match of routeBlockMatches) { | ||
| const index = match.index! | ||
| const length = match[0].length | ||
| s.remove(index, index + length) | ||
| if (macro) | ||
| s.remove(macro.start!, macro.end!) | ||
|
|
There was a problem hiding this comment.
Remove definePage() using absolute offsets; current removal can cut wrong bytes
babelParse positions are relative to the script block, but MagicString operates on the whole SFC string. Compute and add the block’s start offset to avoid corrupting SFCs.
- if (macro)
- s.remove(macro.start!, macro.end!)
+ if (macro) {
+ let base = 0
+ if (sfc.scriptSetup?.loc?.start?.offset != null && sfc.scriptSetup?.content?.includes?.('definePage')) {
+ base = sfc.scriptSetup.loc.start.offset
+ } else if (sfc.script?.loc?.start?.offset != null) {
+ base = sfc.script.loc.start.offset
+ }
+ s.remove(base + (macro.start ?? 0), base + (macro.end ?? 0))
+ }Note: If you later support export default definePage(...) (see findMacro comment), prefer removing the entire containing statement span, not just the call expression.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const s = new MagicString(code) | |
| const routeBlockMatches = s.original.matchAll( | |
| /<route[^>]*>([\s\S]*?)<\/route>/g, | |
| ) | |
| for (const match of routeBlockMatches) { | |
| const index = match.index! | |
| const length = match[0].length | |
| s.remove(index, index + length) | |
| if (macro) | |
| s.remove(macro.start!, macro.end!) | |
| const s = new MagicString(code) | |
| if (macro) { | |
| let base = 0 | |
| if ( | |
| sfc.scriptSetup?.loc?.start?.offset != null && | |
| sfc.scriptSetup?.content?.includes?.('definePage') | |
| ) { | |
| base = sfc.scriptSetup.loc.start.offset | |
| } else if (sfc.script?.loc?.start?.offset != null) { | |
| base = sfc.script.loc.start.offset | |
| } | |
| s.remove(base + (macro.start ?? 0), base + (macro.end ?? 0)) | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/index.ts around lines 112 to 115, the current removal uses
macro.start/macro.end which are positions relative to the script block but
MagicString expects positions in the full SFC; add the script/block start offset
to both start and end before calling s.remove (e.g. compute const offset =
block.start and call s.remove(macro.start + offset, macro.end + offset)); if you
later support export default definePage(...), remove the entire containing
statement span instead of only the call expression.
| export async function tryPageMetaFromMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { | ||
| const sfcScript = sfc.scriptSetup || sfc.script | ||
|
|
||
| if (!sfcScript) { | ||
| return undefined | ||
| } | ||
|
|
||
| const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { | ||
| plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], | ||
| }) | ||
| const macro = findMacro(ast.body, sfc.filename) | ||
| if (macro) { | ||
| const imports = findImports(ast.body).map(imp => babelGenerate(imp).code) | ||
|
|
||
| const [macroOption] = macro.arguments | ||
| const code = babelGenerate(macroOption).code | ||
| let script = '' | ||
| const importScript = imports.join('') | ||
| script += importScript | ||
| script += `export default ${code}` | ||
|
|
||
| const result = await execScript(script, sfc.filename) | ||
| return result as UserPageMeta | ||
| } | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix macro execution to avoid requiring ESM-only deps; stub plugin imports and expose process in VM
Current implementation concatenates import declarations and then executes them under CJS, which requires @uni-helper/vite-plugin-uni-pages and cascades into ast-kit (ESM-only), breaking CI. Build an evaluable script that:
- Preserves top-level code needed by the macro (except the macro call and any default export).
- Drops imports from
@uni-helper/vite-plugin-uni-pagesand injects lightweight stubs forUniPlatformandcurrentUniPlatform. - Keeps other imports as-is (or drop if you want stricter isolation later).
Also ensureexecScript’s VM context includesprocess.
export async function tryPageMetaFromMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> {
const sfcScript = sfc.scriptSetup || sfc.script
if (!sfcScript) {
return undefined
}
- const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', {
- plugins: [['importAttributes', { deprecatedAssertSyntax: true }]],
- })
- const macro = findMacro(ast.body, sfc.filename)
- if (macro) {
- const imports = findImports(ast.body).map(imp => babelGenerate(imp).code)
-
- const [macroOption] = macro.arguments
- const code = babelGenerate(macroOption).code
- let script = ''
- const importScript = imports.join('')
- script += importScript
- script += `export default ${code}`
-
- const result = await execScript(script, sfc.filename)
- return result as UserPageMeta
- }
+ const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', {
+ plugins: [['importAttributes', { deprecatedAssertSyntax: true }]],
+ })
+ const macro = findMacro(ast.body, sfc.filename)
+ if (macro) {
+ if (macro.arguments.length === 0)
+ throw new Error(`definePage() requires one argument: ${sfc.filename}`)
+
+ const pluginId = '@uni-helper/vite-plugin-uni-pages'
+ const allImports = findImports(ast.body)
+ const keptImports = allImports.filter(imp => imp.source.value !== pluginId)
+ const pluginImports = allImports.filter(imp => imp.source.value === pluginId)
+
+ // Preserve all top-level stmts except the macro call stmt and any export default
+ const preserved = ast.body.filter(stmt => {
+ if (t.isExpressionStatement(stmt) && (stmt.expression === macro))
+ return false
+ if (t.isExportDefaultDeclaration(stmt))
+ return false
+ return true
+ })
+
+ // Generate script: stub plugin APIs, re-emit kept imports and preserved code, then export macro arg
+ const aliasBindings: string[] = []
+ for (const imp of pluginImports) {
+ for (const s of imp.specifiers) {
+ if (t.isImportNamespaceSpecifier(s)) {
+ aliasBindings.push(`const ${s.local.name} = { UniPlatform, currentUniPlatform };`)
+ } else if (t.isImportSpecifier(s)) {
+ const imported = (s.imported as t.Identifier).name
+ const local = s.local.name
+ aliasBindings.push(`const ${local} = ${imported};`)
+ } else if (t.isImportDefaultSpecifier(s)) {
+ aliasBindings.push(`const ${s.local.name} = { UniPlatform, currentUniPlatform };`)
+ }
+ }
+ }
+
+ const program = t.program(
+ [
+ // Keep non-plugin imports and other top-level code
+ ...keptImports,
+ ...preserved.filter(n => !t.isImportDeclaration(n)),
+ // export default macroOption
+ t.exportDefaultDeclaration(macro.arguments[0] as t.Expression),
+ ],
+ [],
+ 'module',
+ )
+
+ const preservedCode = babelGenerate(program).code
+ const stubCode = `
+const UniPlatform = {
+ App: 'app',
+ 'App-android': 'app-android', // keep parity with enum values
+ AppAndroid: 'app-android',
+ AppIOS: 'app-ios',
+ Custom: 'custom',
+ H5: 'h5',
+ H5SSR: 'h5:ssr',
+ MpAlipay: 'mp-alipay',
+ MpBaidu: 'mp-baidu',
+ MpJD: 'mp-jd',
+ MpKuaiShou: 'mp-kuaishou',
+ MpLark: 'mp-lark',
+ MpQQ: 'mp-qq',
+ MpTouTiao: 'mp-toutiao',
+ MpWeixin: 'mp-weixin',
+ QuickappWebview: 'quickapp-webview',
+ QuickappWebviewHuawei: 'quickapp-webview-huawei',
+ QuickappWebviewUnion: 'quickapp-webview-union',
+};
+const currentUniPlatform = (platform) => {
+ const env = (typeof process !== 'undefined' && process.env) ? process.env.UNI_PLATFORM : undefined;
+ if (platform === undefined) return env;
+ return env === platform;
+};
+${aliasBindings.join('\n')}
+`
+ const script = `${stubCode}\n${preservedCode}`
+ const result = await execScript(script, sfc.filename)
+ return result as UserPageMeta
+ }
return undefined
}And in execScript (packages/core/src/utils.ts), add process to the VM context so the stub works:
- const vmContext = {
+ const vmContext = {
require: createRequire(dir),
module: {},
exports: {},
__filename: filename,
__dirname: dir,
+ process, // expose Node process to macro stubs
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function tryPageMetaFromMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { | |
| const sfcScript = sfc.scriptSetup || sfc.script | |
| if (!sfcScript) { | |
| return undefined | |
| } | |
| const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { | |
| plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], | |
| }) | |
| const macro = findMacro(ast.body, sfc.filename) | |
| if (macro) { | |
| const imports = findImports(ast.body).map(imp => babelGenerate(imp).code) | |
| const [macroOption] = macro.arguments | |
| const code = babelGenerate(macroOption).code | |
| let script = '' | |
| const importScript = imports.join('') | |
| script += importScript | |
| script += `export default ${code}` | |
| const result = await execScript(script, sfc.filename) | |
| return result as UserPageMeta | |
| } | |
| return undefined | |
| } | |
| // packages/core/src/page.ts | |
| export async function tryPageMetaFromMacro( | |
| sfc: SFCDescriptor, | |
| ): Promise<UserPageMeta | undefined> { | |
| const sfcScript = sfc.scriptSetup || sfc.script | |
| if (!sfcScript) { | |
| return undefined | |
| } | |
| const ast = babelParse( | |
| sfcScript.content, | |
| sfcScript.lang || 'js', | |
| { plugins: [['importAttributes', { deprecatedAssertSyntax: true }]] }, | |
| ) | |
| const macro = findMacro(ast.body, sfc.filename) | |
| if (macro) { | |
| if (macro.arguments.length === 0) | |
| throw new Error(`definePage() requires one argument: ${sfc.filename}`) | |
| const pluginId = '@uni-helper/vite-plugin-uni-pages' | |
| const allImports = findImports(ast.body) | |
| const keptImports = allImports.filter(imp => imp.source.value !== pluginId) | |
| const pluginImports = allImports.filter(imp => imp.source.value === pluginId) | |
| // Preserve all top-level statements except: | |
| // - the definePage(...) call itself | |
| // - any existing default export | |
| const preserved = ast.body.filter(stmt => { | |
| if (t.isExpressionStatement(stmt) && stmt.expression === macro) return false | |
| if (t.isExportDefaultDeclaration(stmt)) return false | |
| return true | |
| }) | |
| // Build stub bindings for plugin imports | |
| const aliasBindings: string[] = [] | |
| for (const imp of pluginImports) { | |
| for (const s of imp.specifiers) { | |
| if (t.isImportNamespaceSpecifier(s) || t.isImportDefaultSpecifier(s)) { | |
| aliasBindings.push( | |
| `const ${s.local.name} = { UniPlatform, currentUniPlatform };`, | |
| ) | |
| } else if (t.isImportSpecifier(s)) { | |
| const imported = (s.imported as t.Identifier).name | |
| aliasBindings.push(`const ${s.local.name} = ${imported};`) | |
| } | |
| } | |
| } | |
| // Rebuild a Program AST: kept imports + preserved code + export default | |
| const program = t.program( | |
| [ | |
| ...keptImports, | |
| ...preserved.filter(n => !t.isImportDeclaration(n)), | |
| t.exportDefaultDeclaration(macro.arguments[0] as t.Expression), | |
| ], | |
| [], | |
| 'module', | |
| ) | |
| const preservedCode = babelGenerate(program).code | |
| const stubCode = ` | |
| const UniPlatform = { | |
| App: 'app', | |
| 'App-android': 'app-android', | |
| AppAndroid: 'app-android', | |
| AppIOS: 'app-ios', | |
| Custom: 'custom', | |
| H5: 'h5', | |
| H5SSR: 'h5:ssr', | |
| MpAlipay: 'mp-alipay', | |
| MpBaidu: 'mp-baidu', | |
| MpJD: 'mp-jd', | |
| MpKuaiShou: 'mp-kuaishou', | |
| MpLark: 'mp-lark', | |
| MpQQ: 'mp-qq', | |
| MpTouTiao: 'mp-toutiao', | |
| MpWeixin: 'mp-weixin', | |
| QuickappWebview: 'quickapp-webview', | |
| QuickappWebviewHuawei: 'quickapp-webview-huawei', | |
| QuickappWebviewUnion: 'quickapp-webview-union', | |
| }; | |
| const currentUniPlatform = (platform) => { | |
| const env = (typeof process !== 'undefined' && process.env) | |
| ? process.env.UNI_PLATFORM | |
| : undefined; | |
| if (platform === undefined) return env; | |
| return env === platform; | |
| }; | |
| ${aliasBindings.join('\n')} | |
| ` | |
| const script = `${stubCode}\n${preservedCode}` | |
| const result = await execScript(script, sfc.filename) | |
| return result as UserPageMeta | |
| } | |
| return undefined | |
| } |
| export async function tryPageMetaFromMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { | |
| const sfcScript = sfc.scriptSetup || sfc.script | |
| if (!sfcScript) { | |
| return undefined | |
| } | |
| const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { | |
| plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], | |
| }) | |
| const macro = findMacro(ast.body, sfc.filename) | |
| if (macro) { | |
| const imports = findImports(ast.body).map(imp => babelGenerate(imp).code) | |
| const [macroOption] = macro.arguments | |
| const code = babelGenerate(macroOption).code | |
| let script = '' | |
| const importScript = imports.join('') | |
| script += importScript | |
| script += `export default ${code}` | |
| const result = await execScript(script, sfc.filename) | |
| return result as UserPageMeta | |
| } | |
| return undefined | |
| } | |
| // packages/core/src/utils.ts | |
| export function execScript(script: string, filename: string): any { | |
| const dir = path.dirname(filename) | |
| const vmContext = { | |
| require: createRequire(dir), | |
| module: {}, | |
| exports: {}, | |
| __filename: filename, | |
| __dirname: dir, | |
| process, // expose Node process to macro stubs | |
| } | |
| // ... | |
| return new VM.Script(script, { filename }) | |
| .runInNewContext(vmContext) | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/page.ts around lines 94-119, the current macro execution
concatenates raw import declarations which causes requiring ESM-only deps;
change the script generation to drop imports from
"@uni-helper/vite-plugin-uni-pages", replace them with lightweight stubs that
export UniPlatform and currentUniPlatform, preserve other non-problematic
top-level code required by the macro (but remove the macro call and any default
export), and build a script that exports the macro option as default for eval;
and in packages/core/src/utils.ts add process to the VM context passed to
execScript so the injected stubs can access process during execution.
| export function currentUniPlatform(): UniPlatform | ||
| export function currentUniPlatform(platform: string): boolean | ||
| export function currentUniPlatform(platform?: string): UniPlatform | boolean { | ||
| return platform === undefined | ||
| ? process.env.UNI_PLATFORM as UniPlatform | ||
| : process.env.UNI_PLATFORM === platform | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix return type and narrow parameter typing for currentUniPlatform
Without an argument, process.env.UNI_PLATFORM can be undefined, but the signature promises UniPlatform. Also accept UniPlatform (not plain string) for stricter checks.
-export function currentUniPlatform(): UniPlatform
-export function currentUniPlatform(platform: string): boolean
-export function currentUniPlatform(platform?: string): UniPlatform | boolean {
- return platform === undefined
- ? process.env.UNI_PLATFORM as UniPlatform
- : process.env.UNI_PLATFORM === platform
-}
+export function currentUniPlatform(): UniPlatform | undefined
+export function currentUniPlatform(platform: UniPlatform): boolean
+export function currentUniPlatform(platform?: UniPlatform): UniPlatform | boolean | undefined {
+ const env = process.env.UNI_PLATFORM as string | undefined
+ if (platform === undefined) {
+ return (Object.values(UniPlatform) as string[]).includes(env ?? '') ? (env as UniPlatform) : undefined
+ }
+ return env === platform
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function currentUniPlatform(): UniPlatform | |
| export function currentUniPlatform(platform: string): boolean | |
| export function currentUniPlatform(platform?: string): UniPlatform | boolean { | |
| return platform === undefined | |
| ? process.env.UNI_PLATFORM as UniPlatform | |
| : process.env.UNI_PLATFORM === platform | |
| } | |
| export function currentUniPlatform(): UniPlatform | undefined | |
| export function currentUniPlatform(platform: UniPlatform): boolean | |
| export function currentUniPlatform(platform?: UniPlatform): UniPlatform | boolean | undefined { | |
| const env = process.env.UNI_PLATFORM as string | undefined | |
| if (platform === undefined) { | |
| return (Object.values(UniPlatform) as string[]).includes(env ?? '') ? (env as UniPlatform) : undefined | |
| } | |
| return env === platform | |
| } |
🤖 Prompt for AI Agents
In packages/core/src/uni-platform.ts around lines 23 to 29, update the overloads
and implementation so the no-arg call can return undefined and the parameter is
narrowed to UniPlatform: change the zero-arg signature to return UniPlatform |
undefined, change the single-arg signature to accept platform: UniPlatform and
return boolean, and in the implementation cast process.env.UNI_PLATFORM to
UniPlatform | undefined and return it when no arg is provided or compare it to
the passed UniPlatform when an arg is given.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/core/src/index.ts (2)
95-103: Guard against empty script contents before parsingAvoid parsing undefined/empty strings; logs help debugging.
- if (sfc.scriptSetup) { - const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') + if (sfc.scriptSetup && typeof sfc.scriptSetup.content === 'string' && sfc.scriptSetup.content.trim() !== '') { + const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') macro = findMacro(ast.body, sfc.filename) } - if (!macro && sfc.script) { - const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') + if (!macro && sfc.script && typeof sfc.script.content === 'string' && sfc.script.content.trim() !== '') { + const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') macro = findMacro(ast.body, sfc.filename) }
113-116: Fix macro removal offsets; current code can delete wrong spans in the SFCmacro.start/end are relative to the script block, but MagicString works on the entire SFC. Add the block’s start offset.
- if (macro) - s.remove(macro.start!, macro.end!) + if (macro) { + let base = 0 + if (sfc.scriptSetup?.loc?.start?.offset != null && sfc.scriptSetup?.content?.includes?.('definePage')) { + base = sfc.scriptSetup.loc.start.offset + } + else if (sfc.script?.loc?.start?.offset != null) { + base = sfc.script.loc.start.offset + } + s.remove(base + (macro.start ?? 0), base + (macro.end ?? 0)) + }packages/core/src/page.ts (2)
137-167: findMacro missesexport default definePage(...)and over-restricts argument typesThis fails on common patterns and blocks identifiers like definePage(meta). Detect ExportDefaultDeclaration and accept any expression; keep duplicate-call protection.
export function findMacro(stmts: t.Statement[], filename: string): t.CallExpression | undefined { const nodes = stmts - .map((raw: t.Node) => { - let node = raw - if (raw.type === 'ExpressionStatement') - node = raw.expression - return isCallOf(node, 'definePage') ? node : undefined - }) + .map((raw: t.Node) => { + let node: t.Node | undefined = raw + if (t.isExpressionStatement(raw)) + node = raw.expression + else if (t.isExportDefaultDeclaration(raw)) + node = raw.declaration as t.Node + return node && isCallOf(node as any, 'definePage') ? (node as t.CallExpression) : undefined + }) .filter((node): node is t.CallExpression => !!node) if (!nodes.length) return - if (nodes.length > 1) { - throw new Error(`每个文件只允许调用一次 definePage(): ${filename}`) - } + if (nodes.length > 1) + throw new Error(`每个文件只允许调用一次 definePage(): ${filename}`) - // 仅第支持一个 definePage + // 仅支持一个 definePage const macro = nodes[0] - // 提取 macro function 内的第一个参数 + // 提取第一个参数 const [opt] = macro.arguments - // 检查 macro 的参数是否正确 - if (opt && !t.isFunctionExpression(opt) && !t.isArrowFunctionExpression(opt) && !t.isObjectExpression(opt)) { - debug.definePage(`definePage() only accept argument in function or object: ${filename}`) - return - } + if (!opt) { + throw new Error(`definePage() missing argument in ${filename}`) + } + if (!t.isExpression(opt)) { + throw new Error(`definePage() argument must be an expression in ${filename}`) + } return macro }
94-119: Critical: VM executes CJS and tries to require ESM deps (ast-kit) from macro imports — fix by stubbing plugin imports and preserving top-level codeCI fails with “require() of ES Module ast-kit” when executing the generated macro script. The current approach concatenates raw import declarations then transpiles to CJS, so imports from '@uni-helper/vite-plugin-uni-pages' cascade into ESM-only deps. Build an evaluable script that:
- Preserves all top-level code except the definePage call and any default export,
- Keeps non-plugin imports,
- Replaces imports from '@uni-helper/vite-plugin-uni-pages' with lightweight in-VM stubs for UniPlatform/currentUniPlatform,
- Exports the macro argument as default.
Apply:
export async function tryPageMetaFromMacro(sfc: SFCDescriptor): Promise<UserPageMeta | undefined> { const sfcScript = sfc.scriptSetup || sfc.script - if (!sfcScript) { + if (!sfcScript || typeof sfcScript.content !== 'string' || sfcScript.content.trim() === '') { return undefined } const ast = babelParse(sfcScript.content, sfcScript.lang || 'js', { plugins: [['importAttributes', { deprecatedAssertSyntax: true }]], }) const macro = findMacro(ast.body, sfc.filename) if (macro) { - const imports = findImports(ast.body).map(imp => babelGenerate(imp).code) - - const [macroOption] = macro.arguments - const code = babelGenerate(macroOption).code - let script = '' - const importScript = imports.join('') - script += importScript - script += `export default ${code}` - - const result = await execScript(script, sfc.filename) - return result as UserPageMeta + if (macro.arguments.length === 0) + throw new Error(`definePage() requires one argument: ${sfc.filename}`) + + const pluginId = '@uni-helper/vite-plugin-uni-pages' + const allImports = findImports(ast.body) + const keptImports = allImports.filter(imp => imp.source.value !== pluginId) + const pluginImports = allImports.filter(imp => imp.source.value === pluginId) + + // preserve all top-level statements except the macro call statement and any export default + const preserved = ast.body.filter(stmt => { + if (t.isExpressionStatement(stmt) && (stmt.expression === macro)) + return false + if (t.isExportDefaultDeclaration(stmt)) + return false + return true + }) + + // stub '@uni-helper/vite-plugin-uni-pages' bindings + const aliasBindings: string[] = [] + for (const imp of pluginImports) { + for (const s of imp.specifiers) { + if (t.isImportNamespaceSpecifier(s) || t.isImportDefaultSpecifier(s)) { + aliasBindings.push(`const ${s.local.name} = { UniPlatform, currentUniPlatform };`) + } else if (t.isImportSpecifier(s)) { + const imported = (s.imported as t.Identifier).name + const local = s.local.name + aliasBindings.push(`const ${local} = ${imported};`) + } + } + } + + const program = t.program( + [ + ...keptImports, + ...preserved.filter(n => !t.isImportDeclaration(n)), + t.exportDefaultDeclaration(macro.arguments[0] as t.Expression), + ], + [], + 'module', + ) + + const preservedCode = babelGenerate(program).code + const stubCode = ` +const UniPlatform = { + App: 'app', + AppAndroid: 'app-android', + AppIOS: 'app-ios', + Custom: 'custom', + H5: 'h5', + H5SSR: 'h5:ssr', + MpAlipay: 'mp-alipay', + MpBaidu: 'mp-baidu', + MpJD: 'mp-jd', + MpKuaiShou: 'mp-kuaishou', + MpLark: 'mp-lark', + MpQQ: 'mp-qq', + MpTouTiao: 'mp-toutiao', + MpWeixin: 'mp-weixin', + QuickappWebview: 'quickapp-webview', + QuickappWebviewHuawei: 'quickapp-webview-huawei', + QuickappWebviewUnion: 'quickapp-webview-union', +}; +const currentUniPlatform = (platform) => { + const env = (typeof process !== 'undefined' && process.env) ? process.env.UNI_PLATFORM : undefined; + if (platform === undefined) return env; + return env === platform; +}; +${aliasBindings.join('\n')} +` + const script = `${stubCode}\n${preservedCode}` + const result = await execScript(script, sfc.filename) + return result as UserPageMeta } return undefined }Follow-up needed in utils.ts to expose process to the VM (see separate comment below).
🧹 Nitpick comments (10)
.vscode/settings.json (1)
24-34: Review .vscode/settings.json – prune unused ESLint validatorsI ran a workspace-wide search for files matching the extra extensions you enabled under
eslint.validateand found only one SCSS file (packages/playground/src/uni.scss). No.astro,.svelte,.gql/.graphql,.xml,.less,.pcssor.postcssfiles exist. Keeping unused validators adds startup overhead and may surface irrelevant errors.Please trim the array to the languages your repo actually contains. At minimum, you can drop:
• "xml"
• "gql"
• "graphql"
• "astro"
• "svelte"
• "less"
• "pcss"
• "postcss"You’ll likely want to keep
"css"(if you have any.cssfiles) and"scss"for your existing SCSS, and only include"toml"if you’re editing.tomlfiles in this workspace.package.json (1)
34-43: Add a Node engines field to enforce ESLint v9’s minimum Node version
ESLint v9.x drops support for Node.js < 18.18.0; declaring an “engines” field in the root package.json will warn if anyone attempts to install or run on an older Node version (eslint.org).Apply:
{ "name": "@vite-plugin-uni-pages/monorepo", "type": "module", "version": "0.3.2", "private": true, "packageManager": "[email protected]", + "engines": { + "node": ">=18.18.0" + }, "description": "Use TypeScript to write pages.json of uni-app",Key points:
- Your
.nvmrcpins to Node 20.x (latest patch will always be ≥ 20.9.0).- The CI matrix runs on Node 18.x and 20.x, and
actions/setup-node@…with “18”/“20” picks the latest patch releases (so ≥ 18.18.0 and ≥ 20.9.0).This ensures both local and CI environments satisfy ESLint v9’s minimum Node requirement and surfaces a warning if someone is on an older version.
packages/core/src/customBlock.ts (2)
8-8: Fix import order per lint rule (perfectionist/sort-imports)Swap sibling imports to satisfy the linter.
-import { debug } from './utils' -import type { CustomBlock, RouteBlockLang } from './types' +import type { CustomBlock, RouteBlockLang } from './types' +import { debug } from './utils'
15-16: Normalize route block lang to lowercase to avoid case-sensitive parse missesIf users write
<route lang="YAML">or similar, current equality checks fail.- const lang = block.lang ?? routeBlockLang + const lang = (block.lang ?? routeBlockLang).toLowerCase() as RouteBlockLangpackages/core/src/index.ts (3)
6-11: Resolve lint errors: sort imports and drop unused kolorist exportsUnused lightYellow/link and import ordering are failing lint. Clean up and reorder per config.
-import { createLogger } from 'vite' -import { babelParse } from 'ast-kit' +import { babelParse } from 'ast-kit' +import { createLogger } from 'vite' import MagicString from 'magic-string' -import chokidar from 'chokidar' -import type { CallExpression } from '@babel/types' -import { lightYellow, link } from 'kolorist' +import chokidar from 'chokidar' +import type { CallExpression } from '@babel/types' +// removed unused kolorist imports import type { UserOptions } from './types' import { PageContext } from './context'
88-91: Gate on full SFC only; ignore sub-requests with query stringsTransform will also receive ids like foo.vue?vue&type=script... Parsing those as full SFC is wrong. Early-return when a query is present; also avoid false positives on files ending with uvue etc. by stripping query first.
- if (!FILE_EXTENSIONS.find(ext => id.endsWith(ext))) { + const [file] = id.split('?', 2) + if (!FILE_EXTENSIONS.find(ext => file.endsWith(ext))) { return null } + // Ignore sub-requests (script/style/template blocks) + if (id.includes('?')) + return null
110-112: Consistent error messaging (i18n) and actionable contextThe mixed usage error is good; consider an English variant and filename for CI logs.
- throw new Error(`不支持混合使用 definePage() 和 <route/> `) + throw new Error(`混合使用 definePage() 和 <route/> 不被支持 (file: ${id}). Mixed usage of definePage() and <route/> is not supported.`)packages/core/src/page.ts (3)
62-72: Consider enforcing mutual exclusivity here too (macro vs ) for consistencyIndex transform already throws when both appear. Mirroring that check at read time would surface a clearer error during metadata generation as well.
const sfc = parseSFC(content, { filename: this.path.absolutePath }) - const meta = await tryPageMetaFromMacro(sfc) - if (meta) { - return meta - } - return tryPageMetaFromCustomBlock(sfc, this.ctx.options.routeBlockLang) + const macroMeta = await tryPageMetaFromMacro(sfc) + const hasRoute = !!getRouteSfcBlock(sfc) + if (macroMeta && hasRoute) + throw new Error(`混合使用 definePage() 和 <route/> 不被支持 (file: ${this.path.relativePath}). Mixed usage is not supported.`) + if (macroMeta) + return macroMeta + return tryPageMetaFromCustomBlock(sfc, this.ctx.options.routeBlockLang)
68-108: Addprocessto VM context to support currentUniPlatform stub and env checksRequired by the stub in tryPageMetaFromMacro; change is outside this file (utils.ts).
Add in packages/core/src/utils.ts:
const vmContext = { require: createRequire(dir), module: {}, exports: {}, __filename: filename, __dirname: dir, + process, // expose Node process for stubs and user code }Want me to open a follow-up PR with this and tests?
39-60: Improve error chaining for easier debuggingYou already wrap with filename; preserving original stack helps triage.
- catch (err: any) { - throw new Error(`Read page options fail in ${this.path.relativePath}\n${err.message}`) - } + catch (err: any) { + const e = new Error(`Read page options fail in ${this.path.relativePath}\n${err?.message ?? err}`) + ;(e as any).cause = err + throw e + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.vscode/settings.json(1 hunks)package.json(1 hunks)packages/core/package.json(2 hunks)packages/core/src/customBlock.ts(2 hunks)packages/core/src/index.ts(3 hunks)packages/core/src/page.ts(1 hunks)packages/playground/package.json(1 hunks)packages/playground/src/pages/define-page/utils.ts(1 hunks)packages/schema/schema.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/schema/schema.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/playground/package.json
- packages/playground/src/pages/define-page/utils.ts
- packages/core/package.json
🧰 Additional context used
🧬 Code graph analysis (3)
packages/core/src/page.ts (4)
packages/core/src/context.ts (3)
PageContext(29-377)PageContext(32-365)page(205-205)packages/core/src/types.ts (4)
PagePath(120-123)PageMetaDatum(125-140)UserPageMeta(147-153)RouteBlockLang(16-16)packages/core/src/utils.ts (3)
babelGenerate(115-115)execScript(69-109)debug(23-33)packages/core/src/customBlock.ts (2)
getRouteSfcBlock(69-71)getRouteBlock(73-77)
packages/core/src/customBlock.ts (3)
packages/core/src/types.ts (2)
RouteBlockLang(16-16)CustomBlock(7-10)test/parser.spec.ts (4)
path(16-36)it(15-83)path(38-60)path(62-82)packages/core/src/utils.ts (1)
hasChanged(75-80)
packages/core/src/index.ts (2)
packages/core/src/constant.ts (1)
FILE_EXTENSIONS(6-6)packages/core/src/page.ts (2)
parseSFC(75-92)findMacro(137-167)
🪛 GitHub Check: test (windows-latest, 18)
packages/core/src/page.ts
[failure] 58-58: test/generate.spec.ts > generate routes > vue - not merge pages snapshot
Error: Read page options fail in ..\packages\playground\src\pages\define-page\conditional-compilation.vue
D:/a/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\node_modules.pnpm\[email protected]\node_modules\ast-kit\dist\index.js from D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs not supported.
Instead change the require of index.js in D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:174:5
[failure] 58-58: test/generate.spec.ts > generate routes > vue - pages snapshot
Error: Read page options fail in ..\packages\playground\src\pages\define-page\conditional-compilation.vue
D:/a/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\node_modules.pnpm\[email protected]\node_modules\ast-kit\dist\index.js from D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs not supported.
Instead change the require of index.js in D:\a\vite-plugin-uni-pages\vite-plugin-uni-pages\packages\core\dist\index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:28:5
🪛 GitHub Check: test (macos-latest, 18)
packages/core/src/page.ts
[failure] 58-58: test/generate.spec.ts > generate routes > vue - not merge pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:174:5
[failure] 58-58: test/generate.spec.ts > generate routes > vue - pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /Users/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:28:5
🪛 GitHub Check: test (ubuntu-latest, 18)
packages/core/src/page.ts
[failure] 58-58: test/generate.spec.ts > generate routes > vue - not merge pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:174:5
[failure] 58-58: test/generate.spec.ts > generate routes > vue - pages snapshot
Error: Read page options fail in ../packages/playground/src/pages/define-page/conditional-compilation.vue
/home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/playground/src/pages/define-page/conditional-compilation.vue: require() of ES Module /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/node_modules/.pnpm/[email protected]/node_modules/ast-kit/dist/index.js from /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs not supported.
Instead change the require of index.js in /home/runner/work/vite-plugin-uni-pages/vite-plugin-uni-pages/packages/core/dist/index.cjs to a dynamic import() which is available in all CommonJS modules.
❯ Page.readPageMeta packages/core/src/page.ts:58:13
❯ Page.getPageMeta packages/core/src/page.ts:29:7
❯ PageContext.parsePages packages/core/src/context.ts:206:35
❯ PageContext.mergePageMetaData packages/core/src/context.ts:249:26
❯ test/generate.spec.ts:28:5
🪛 ESLint
packages/core/src/page.ts
[error] 4-4: Expected "comment-json" to come before "vite".
(perfectionist/sort-imports)
[error] 5-5: Expected "@babel/types" to come before "comment-json".
(perfectionist/sort-imports)
[error] 7-7: Expected "@vue/compiler-sfc" (type) to come before "ast-kit" (external).
(perfectionist/sort-imports)
[error] 10-10: Expected "./types" (sibling-type) to come before "./utils" (sibling).
(perfectionist/sort-imports)
[error] 11-11: Expected "./context" to come before "./types".
(perfectionist/sort-imports)
packages/core/src/customBlock.ts
[error] 8-8: Expected "./types" (sibling-type) to come before "./utils" (sibling).
(perfectionist/sort-imports)
packages/core/src/index.ts
[error] 6-6: Expected "ast-kit" to come before "vite".
(perfectionist/sort-imports)
[error] 8-8: Expected "chokidar" to come before "magic-string".
(perfectionist/sort-imports)
[error] 9-9: Expected "@babel/types" (type) to come before "chokidar" (external).
(perfectionist/sort-imports)
[error] 10-10: 'lightYellow' is defined but never used.
(unused-imports/no-unused-imports)
[error] 10-10: 'link' is defined but never used.
(unused-imports/no-unused-imports)
[error] 11-11: Expected "./types" (sibling-type) to come before "kolorist" (external).
(perfectionist/sort-imports)
[error] 13-18: Expected "./constant" to come before "./context".
(perfectionist/sort-imports)
🪛 GitHub Actions: CI
packages/core/src/page.ts
[error] 58-58: Read page options fail in conditional-compilation.vue. ES Module 'ast-kit' cannot be required from CommonJS; use dynamic import() as suggested by the error trace. (Step: vitest)
[error] 29-29: Page.getPageMeta failed while reading page options for conditional-compilation.vue. (Step: vitest)
🔇 Additional comments (3)
packages/core/src/customBlock.ts (1)
69-77: API shape looks good; pure extractor and explicit lang make this deterministicThe new descriptor-based getRouteSfcBlock/getRouteBlock pair removes IO and improves composability. No issues spotted.
packages/core/src/page.ts (2)
27-55: Page-level caching and change detection look solidgetPageMeta/hasChanged with rawOptions hash keeps recompute cheap and stable. Nice.
121-135: Custom block fallback path looks correct and type-safeDefaults type to 'page' and merges JSONC/YAML content via comment-json assign. Good.
| import { readFileSync } from 'node:fs' | ||
| import { extname } from 'node:path' | ||
| import { normalizePath } from 'vite' | ||
| import { assign as cjAssign } from 'comment-json' | ||
| import * as t from '@babel/types' | ||
| import { babelParse, isCallOf } from 'ast-kit' | ||
| import type { SFCDescriptor, SFCParseOptions } from '@vue/compiler-sfc' | ||
| import { parse as VueParser } from '@vue/compiler-sfc' | ||
| import { babelGenerate, debug, execScript } from './utils' | ||
| import type { PageMetaDatum, PagePath, RouteBlockLang, UserPageMeta } from './types' | ||
| import type { PageContext } from './context' | ||
| import { getRouteBlock, getRouteSfcBlock } from './customBlock' | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Sort imports per lint and remove unused symbols if any arise after refactor
The linter flags order; after the macro changes, re-run autofix.
🏁 Script executed:
#!/bin/bash
pnpm -w lint --fix || true
rg -n "no-unused" -n node_modules -S .eslintrc* . || trueLength of output: 397
Fix README.md parsing error before running ESLint autofix
There’s a blocking lint error in your repo that prevents pnpm -w lint --fix from completing successfully:
- README.md (line 144:19): “Parsing error: Property or signature expected” is causing ESLint to fail. Please correct the Markdown syntax (or update your ESLint ignore patterns) so that linting can run through the entire codebase.
Once the parsing error is resolved:
- Re-run
pnpm -w lint --fix(or target specific files, e.g.packages/core/src/page.ts) to apply import sorting and remove any unused symbols. - Verify that in
packages/core/src/page.tsimports are ordered per your ESLint rules and that no unused imports remain.
🧰 Tools
🪛 ESLint
[error] 4-4: Expected "comment-json" to come before "vite".
(perfectionist/sort-imports)
[error] 5-5: Expected "@babel/types" to come before "comment-json".
(perfectionist/sort-imports)
[error] 7-7: Expected "@vue/compiler-sfc" (type) to come before "ast-kit" (external).
(perfectionist/sort-imports)
[error] 10-10: Expected "./types" (sibling-type) to come before "./utils" (sibling).
(perfectionist/sort-imports)
[error] 11-11: Expected "./context" to come before "./types".
(perfectionist/sort-imports)
🤖 Prompt for AI Agents
In README.md around line 144:19, fix the Markdown syntax causing "Parsing error:
Property or signature expected" (e.g., unclosed code fence, stray backtick/brace
or malformed TypeScript/JS snippet) so ESLint can parse the file; after fixing
README, run pnpm -w lint --fix (or pnpm -w lint --fix --filter packages/core)
and then in packages/core/src/page.ts (lines 1–13) ensure imports are sorted per
project ESLint rules and remove any unused imports (verify extname,
normalizePath, cjAssign, t, babelParse/isCallOf, SFCDescriptor/SFCParseOptions,
VueParser, babelGenerate/debug/execScript,
PageMetaDatum/PagePath/RouteBlockLang/UserPageMeta, PageContext,
getRouteBlock/getRouteSfcBlock are actually used and drop those that are not).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
packages/core/src/index.ts (2)
92-104: Add content/null checks and safe parsing for script blocks.scriptSetup/script can exist without content. Parsing an empty/undefined string will throw. Wrap parsing in guards and catch parse errors to avoid breaking the transform for a single file.
Apply this diff:
- const sfc = parseSFC(code, { filename: id }) - - let macro: CallExpression | undefined - if (sfc.scriptSetup) { - const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang || 'js') - macro = findMacro(ast.body, sfc.filename) - } - - if (!macro && sfc.script) { - const ast = babelParse(sfc.script.content, sfc.script.lang || 'js') - macro = findMacro(ast.body, sfc.filename) - } + const sfc = parseSFC(code, { filename: id }) + + let macro: CallExpression | undefined + try { + if (sfc.scriptSetup?.content && sfc.scriptSetup.content.trim()) { + const ast = babelParse(sfc.scriptSetup.content, sfc.scriptSetup.lang ?? 'js') + macro = findMacro(ast.body, sfc.filename) + } + if (!macro && sfc.script?.content && sfc.script.content.trim()) { + const ast = babelParse(sfc.script.content, sfc.script.lang ?? 'js') + macro = findMacro(ast.body, sfc.filename) + } + } + catch (e) { + ctx.logger?.warn(`[vite-plugin-uni-pages] Failed to parse ${id}: ${String(e)}`) + }
114-116: Remove definePage() using block-relative offsets; current removal can cut wrong bytes.Babel node positions are relative to the parsed block, but MagicString expects offsets in the whole SFC. Add the script block’s start offset. Also guard when start/end are missing.
Apply this diff:
- if (macro) - s.remove(macro.start!, macro.end!) + if (macro) { + let base = 0 + // Heuristic: if scriptSetup contains definePage, prefer its offset + if (sfc.scriptSetup?.loc?.start?.offset != null && sfc.scriptSetup?.content?.includes?.('definePage')) + base = sfc.scriptSetup.loc.start.offset + else if (sfc.script?.loc?.start?.offset != null) + base = sfc.script.loc.start.offset + + const start = macro.start ?? 0 + const end = macro.end ?? start + s.remove(base + start, base + end) + }Note: In the future, consider removing the entire containing statement span (e.g., ExpressionStatement) to avoid leaving stray semicolons.
🧹 Nitpick comments (4)
packages/core/src/index.ts (4)
6-11: Fix import order to satisfy ESLint (perfectionist/sort-imports).ESLint hints indicate ordering issues among external, type, and sibling imports. Reorder as below to address:
- Place ast-kit before vite imports.
- Place the type-only @babel/types import before chokidar.
- Place chokidar before magic-string.
- Place the sibling type import ./types before the external kolorist.
- Place ./constant before ./context.
Apply this diff:
-import type { Plugin } from 'vite' -import { createLogger } from 'vite' -import { babelParse } from 'ast-kit' -import MagicString from 'magic-string' -import chokidar from 'chokidar' -import type { CallExpression } from '@babel/types' -import { bold, dim, lightYellow, link } from 'kolorist' -import type { UserOptions } from './types' +import { babelParse } from 'ast-kit' +import type { CallExpression } from '@babel/types' +import chokidar from 'chokidar' +import MagicString from 'magic-string' +import type { Plugin } from 'vite' +import { createLogger } from 'vite' +import type { UserOptions } from './types' +import { bold, dim, lightYellow, link } from 'kolorist' -import { PageContext } from './context' -import { - FILE_EXTENSIONS, - MODULE_ID_VIRTUAL, - OUTPUT_NAME, - RESOLVED_MODULE_ID_VIRTUAL, -} from './constant' +import { + FILE_EXTENSIONS, + MODULE_ID_VIRTUAL, + OUTPUT_NAME, + RESOLVED_MODULE_ID_VIRTUAL, +} from './constant' +import { PageContext } from './context'Tip: running your repo’s lint fixer (e.g., pnpm lint --fix) should align the final order with your exact rule config.
Also applies to: 13-18
110-112: Make the mixed-usage error bilingual for dev UX.Optional, but a bilingual message reduces confusion in mixed-language teams.
Apply this diff:
- if (macro && routeBlock) - throw new Error(`不支持混合使用 definePage() 和 <route/> ${id}`) + if (macro && routeBlock) + throw new Error(`不支持混合使用 definePage() 和 <route/> / Mixing definePage() with <route/> is not supported: ${id}`)
117-131: Use Vite’s logger instead of console for warnings, and scope the message.console.log in transforms can be noisy and hard to filter. Use ctx.logger to integrate with Vite logging, and keep the deprecation message concise.
Apply this diff:
- if (routeBlock) { - // eslint-disable-next-line no-console - console.log(lightYellow('警告：'), `${bold('<route/>')} 标签已废弃，将在下一个版本中移除，请使用 definePage() 代替；${link('查看迁移指南', 'https://uni-helper.js.org/vite-plugin-uni-pages/definePage')}。`) - // eslint-disable-next-line no-console - console.log(dim(id)) - // eslint-disable-next-line no-console - console.log() + if (routeBlock) { + ctx.logger?.warn( + `${lightYellow('Deprecation:')} ${bold('<route/>')} is deprecated and will be removed in the next version. Please use definePage() instead. ${link('Migration guide', 'https://uni-helper.js.org/vite-plugin-uni-pages/definePage')}\n${dim(id)}`, + ) const routeBlockMatches = s.original.matchAll( /<route[^>]*>([\s\S]*?)<\/route>/g, ) for (const match of routeBlockMatches) { const index = match.index! const length = match[0].length s.remove(index, index + length) } }
105-109: Short-circuit early-return is good, but consider aligning the predicate with future sources.Route-block presence gate looks right. If you later support other sources (e.g., virtual blocks), centralize this check behind a helper to keep transform lean.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/core/src/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/index.ts (2)
packages/core/src/constant.ts (1)
FILE_EXTENSIONS(6-6)packages/core/src/page.ts (2)
parseSFC(75-92)findMacro(137-167)
🪛 ESLint
packages/core/src/index.ts
[error] 6-6: Expected "ast-kit" to come before "vite".
(perfectionist/sort-imports)
[error] 8-8: Expected "chokidar" to come before "magic-string".
(perfectionist/sort-imports)
[error] 9-9: Expected "@babel/types" (type) to come before "chokidar" (external).
(perfectionist/sort-imports)
[error] 11-11: Expected "./types" (sibling-type) to come before "kolorist" (external).
(perfectionist/sort-imports)
[error] 13-18: Expected "./constant" to come before "./context".
(perfectionist/sort-imports)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (2)
packages/core/src/index.ts (2)
30-31: Re-exporting page and uni-platform from core index is good API surface hygiene.This aligns the public API with the new modules and keeps consumers’ imports stable.
134-142: Sourcemap generation LGTM.Includes content and sets source file; good for DX.
| if (!FILE_EXTENSIONS.find(ext => id.endsWith(ext))) { | ||
| return null | ||
| } | ||
|
|
There was a problem hiding this comment.
Guard transform by extension after cleaning Vite query/hash from id.
Vite passes ids like Foo.vue?vue&type=script. The current endsWith check will miss those and skip the transform. Also, prefer .some for clarity over .find for boolean intent.
Apply this diff:
- if (!FILE_EXTENSIONS.find(ext => id.endsWith(ext))) {
- return null
- }
+ // strip query/hash to correctly match .vue-like files
+ const cleanId = id.split('?', 1)[0].split('#', 1)[0]
+ if (!FILE_EXTENSIONS.some(ext => cleanId.endsWith(`.${ext}`)))
+ return null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!FILE_EXTENSIONS.find(ext => id.endsWith(ext))) { | |
| return null | |
| } | |
| // strip query/hash to correctly match .vue-like files | |
| const cleanId = id.split('?', 1)[0].split('#', 1)[0] | |
| if (!FILE_EXTENSIONS.some(ext => cleanId.endsWith(`.${ext}`))) | |
| return null |
🤖 Prompt for AI Agents
In packages/core/src/index.ts around lines 88 to 91, the extension guard uses
id.endsWith against raw Vite ids (which may include query/hash like
?vue&type=script or #fragment) and uses .find; strip any query string and hash
from the id first (e.g., drop everything from the first '?' or '#'), then test
the cleaned id against FILE_EXTENSIONS using .some for a boolean check; update
the guard to operate on the cleaned id so files such as Foo.vue?vue&type=script
are matched correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/playground/src/pages/test-jsonc-with-comment.vue (1)
14-16: H5 pull-down refresh likely requires h5.pullToRefresh instead of enablePullDownRefresh.On H5, uni-app typically enables pull-to-refresh via the h5.pullToRefresh object rather than the top-level enablePullDownRefresh flag. If this sample is meant to reflect a realistic H5 config (not just parser retention), consider switching to the H5-specific shape. Otherwise, please confirm the current shape is intentional for test coverage only.
Example adjustment within the route block:
- // #ifdef H5 - "enablePullDownRefresh": true - // #endif + // #ifdef H5 + "h5": { + "pullToRefresh": { + // customize if needed + } + } + // #endiftest/generate.spec.ts (2)
101-163: DefinePage entries omit "type": ensure consistency or document the implicit default.Existing routes frequently include
"type": "page", but newly added define-page routes don’t. If"type"is now optional with a default of"page", consider documenting this or updating snapshots to consistently include it to reduce ambiguity.If you prefer explicitness, adjust generator output (or the snapshot fixture) to include
"type": "page"for define-page routes.
257-319: Same concerns as above for define-page routes in the non-merge snapshot.
- Missing
"type": "page"fields (if expected).- Presence of
"tabbar": {}(if not plugin-specific).- Ensure route ordering remains deterministic across OS/filesystems to keep snapshots stable.
If ordering might vary, consider sorting by path before snapshotting to reduce flakiness.
test/parser.spec.ts (1)
1-4: Fix import order to satisfy ESLint (perfectionist/sort-imports).Reorder built-in and package imports to match the linter’s expectations.
Apply:
-import { resolve } from 'node:path' -import { readFileSync } from 'node:fs' -import { describe, expect, it } from 'vitest' -import { stringify as cjStringify } from 'comment-json' +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' +import { stringify as cjStringify } from 'comment-json' +import { describe, expect, it } from 'vitest'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/playground/src/pages/test-jsonc-with-comment.vue(1 hunks)test/generate.spec.ts(4 hunks)test/parser.spec.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/playground/src/pages/test-jsonc-with-comment.vue (1)
packages/core/src/config/types/globalStyle/h5.ts (1)
H5(3-143)
test/generate.spec.ts (2)
packages/core/src/context.ts (1)
updatePagesJSON(281-342)packages/core/src/index.ts (1)
VitePluginUniPages(41-117)
test/parser.spec.ts (2)
packages/core/src/customBlock.ts (3)
getRouteSfcBlock(88-95)getRouteBlock(97-101)parseSFC(12-27)packages/core/src/utils.ts (1)
hasChanged(75-80)
🪛 ESLint
test/parser.spec.ts
[error] 2-2: Expected "node:fs" to come before "node:path".
(perfectionist/sort-imports)
[error] 4-4: Expected "comment-json" to come before "vitest".
(perfectionist/sort-imports)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (6)
packages/playground/src/pages/test-jsonc-with-comment.vue (1)
10-15: Switching conditional compilation to H5 looks correct.The updated guards and title align with the H5 platform intent and the updated snapshots.
test/generate.spec.ts (3)
66-68: Snapshot updates for H5 gating and title: good.The H5-specific conditional comments and title are reflected as expected in both snapshots.
Also applies to: 220-222
70-71: Possible inconsistency: top-level enablePullDownRefresh appears unguarded while style keeps H5 guards.Source SFC wraps enablePullDownRefresh with // #ifdef H5, but the generated routes snapshot shows the property without guards, whereas style retains comments. Please confirm whether the merge/serialize path intentionally strips platform guards for top-level props but preserves them inside style, or if this is an unintended divergence.
If unintended, consider normalizing handling so comments/guards are preserved or stripped consistently across all properties.
Also applies to: 224-225
136-145: "tabbar" vs "tabBar": validate intended key.Uni-app’s global config uses
tabBar(camel-cased, global scope). This snapshot shows"tabbar": {}on a page entry. If this is a plugin-specific meta, carry on; otherwise, confirm casing/placement to avoid confusion or accidental no-ops.Would you like me to add a small assertion to tests that verifies unknown keys are either preserved by design or normalized/dropped by the generator?
test/parser.spec.ts (2)
20-23: Descriptor-based route-block extraction is a solid move.Passing an SFC descriptor decouples parsing from FS access and improves testability. Nice.
20-23: No async/await needed for parseSFC callsConfirmed that parseSFC is declared as a synchronous function returning SFCDescriptor in packages/core/src/page.ts (line 75) and is not marked async or returning a Promise . All existing call sites—in tests (e.g. test/parser.spec.ts at lines 20–23, 43–46, 68–71) and in code (packages/core/src/page.ts:64; packages/core/src/index.ts:92)—invoke parseSFC synchronously, so no changes are required.
2 participants
related
Description 描述
Linked Issues 关联的 Issues
Additional context 额外上下文
Summary by CodeRabbit
New Features
Playground
Tests
Documentation
Chores