Skip to content

Commit 11191bc

Browse files
committed
fix: track definePage imports per-file to fix named view race condition
Fix #2670
1 parent 8e5f147 commit 11191bc

5 files changed

Lines changed: 63 additions & 11 deletions

File tree

packages/router/src/unplugin/codegen/generateRouteRecords.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ ${node
3636

3737
const definePageDataList: string[] = []
3838

39-
if (node.hasDefinePage) {
39+
if (node.needsDefinePageImport) {
4040
for (const [name, filePath] of node.value.components) {
41+
if (!node.fileNeedsDefinePageImport(filePath)) continue
4142
const pageDataImport = `_definePage_${name}_${importsMap.size}`
4243
definePageDataList.push(pageDataImport)
4344
const lang = getLang(filePath)

packages/router/src/unplugin/codegen/generateRouteResolver.spec.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,8 +1086,8 @@ describe('generateRouteResolver', () => {
10861086
tree.insert('profile', 'profile.vue')
10871087
const profileNode = tree.children.get('profile')!
10881088

1089-
// Mark it as having definePage (this would normally be set by the plugin when parsing the file)
1090-
profileNode.hasDefinePage = true
1089+
// Mark it as having definePage with runtime properties (this would normally be set by the plugin when parsing the file)
1090+
profileNode.setDefinePageImport('profile.vue', true)
10911091

10921092
const resolver = generateRouteResolver(
10931093
tree,
@@ -1120,6 +1120,27 @@ describe('generateRouteResolver', () => {
11201120
`)
11211121
})
11221122

1123+
it('only imports definePage for files that need it in named views', () => {
1124+
const tree = new PrefixTree(DEFAULT_OPTIONS)
1125+
tree.insert('dashboard', 'dashboard.vue')
1126+
tree.insert('dashboard@sidebar', 'dashboard@sidebar.vue')
1127+
const dashboardNode = tree.children.get('dashboard')!
1128+
1129+
// Only the default view has definePage with runtime properties (e.g. meta)
1130+
dashboardNode.setDefinePageImport('dashboard.vue', true)
1131+
1132+
const resolver = generateRouteResolver(
1133+
tree,
1134+
DEFAULT_OPTIONS,
1135+
new ImportsMap(),
1136+
new Map()
1137+
)
1138+
1139+
// Should only import ?definePage for dashboard.vue, not dashboard@sidebar.vue
1140+
expect(resolver).toContain('_definePage_default_')
1141+
expect(resolver).not.toContain('_definePage_sidebar_')
1142+
})
1143+
11231144
it('includes query property in route records with query params', () => {
11241145
const tree = new PrefixTree(DEFAULT_OPTIONS)
11251146
tree.insert('search', 'search.vue')

packages/router/src/unplugin/codegen/generateRouteResolver.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ export function generateRouteRecord({
153153

154154
// Handle definePage imports
155155
const definePageDataList: string[] = []
156-
// TODO: optimize to only add the record merge when needed
157-
if (node.hasDefinePage) {
156+
if (node.needsDefinePageImport) {
158157
for (const [name, filePath] of node.value.components) {
158+
if (!node.fileNeedsDefinePageImport(filePath)) continue
159159
const pageDataImport = `_definePage_${name}_${importsMap.size}`
160160
definePageDataList.push(pageDataImport)
161161
const lang = getLang(filePath)

packages/router/src/unplugin/core/context.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -170,8 +170,10 @@ export function createRoutesContext(options: ResolvedOptions) {
170170
const content = await fs.readFile(filePath, 'utf8')
171171
// TODO: track if it changed and to not always trigger HMR
172172
const definedPageInfo = extractDefinePageInfo(content, filePath)
173-
// TODO: add a test that this can be set to true and then to false
174-
node.hasDefinePage = definedPageInfo?.hasRemainingProperties ?? false
173+
node.setDefinePageImport(
174+
filePath,
175+
definedPageInfo?.hasRemainingProperties ?? false
176+
)
175177
// TODO: track if it changed and if generateRoutes should be called again
176178
const routeBlock = getRouteBlock(filePath, content, options)
177179
// TODO: should warn if hasDefinePage and customRouteBlock

packages/router/src/unplugin/core/tree.ts

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,38 @@ export class TreeNode {
5555
*/
5656
options: TreeNodeOptions
5757

58-
// FIXME: refactor this code. It currently helps to keep track if a page has at least one component with `definePage()` but it doesn't tell which. It should keep track of which one while still caching the result per file.
5958
/**
60-
* Should this page import the page info
59+
* Set of file paths that use `definePage()` with runtime properties (meta, props, etc.) that require a `?definePage`
60+
* import at build time. Tracked per-file to avoid race conditions when multiple files (e.g. named views) map to the
61+
* same node.
6162
*/
62-
hasDefinePage: boolean = false
63+
private _needsDefinePageImport: Set<string> = new Set()
64+
65+
/**
66+
* Whether at least one component file uses `definePage()` with runtime properties (meta, props, etc.) that require a
67+
* `?definePage` import at build time.
68+
*/
69+
get needsDefinePageImport(): boolean {
70+
return this._needsDefinePageImport.size > 0
71+
}
72+
73+
/**
74+
* Mark whether a file needs a `?definePage` import.
75+
*/
76+
setDefinePageImport(filePath: string, needsImport: boolean) {
77+
if (needsImport) {
78+
this._needsDefinePageImport.add(filePath)
79+
} else {
80+
this._needsDefinePageImport.delete(filePath)
81+
}
82+
}
83+
84+
/**
85+
* Check if a specific file needs a `?definePage` import.
86+
*/
87+
fileNeedsDefinePageImport(filePath: string): boolean {
88+
return this._needsDefinePageImport.has(filePath)
89+
}
6390

6491
/**
6592
* Creates a new tree node.
@@ -510,7 +537,7 @@ export class TreeNode {
510537
!this.value.components.get('default'))
511538
? ` ⎈(${Array.from(this.value.components.keys()).join(', ')})`
512539
: ''
513-
}${this.hasDefinePage ? ' ⚑ definePage()' : ''}`
540+
}${this.needsDefinePageImport ? ' ⚑ definePage()' : ''}`
514541
}
515542

516543
/**
@@ -572,6 +599,7 @@ export class PrefixTree extends TreeNode {
572599
}
573600
}
574601

602+
node.setDefinePageImport(filePath, false)
575603
this.map.delete(filePath)
576604

577605
if (node.children.size === 0 && node.value.components.size === 0) {

0 commit comments

Comments
 (0)