diff --git a/playground/typed-router.d.ts b/playground/typed-router.d.ts index c4b6e56f2..9204bb422 100644 --- a/playground/typed-router.d.ts +++ b/playground/typed-router.d.ts @@ -64,6 +64,7 @@ declare module 'vue-router/auto-routes' { '/partial-[name]': RouteRecordInfo<'/partial-[name]', '/partial-:name', { name: ParamValue }, { name: ParamValue }>, '/custom-path': RouteRecordInfo<'/custom-path', '/surprise-:id(\d+)', Record, Record>, '/test-[a-id]': RouteRecordInfo<'/test-[a-id]', '/test-:a-id', { aId: ParamValue }, { aId: ParamValue }>, + '/test-broken-syntax': RouteRecordInfo<'/test-broken-syntax', '/test-broken-syntax', Record, Record>, '/todos/': RouteRecordInfo<'/todos/', '/todos', Record, Record>, '/todos/+layout': RouteRecordInfo<'/todos/+layout', '/todos/+layout', Record, Record>, '/users/': RouteRecordInfo<'/users/', '/users', Record, Record>, @@ -287,6 +288,10 @@ declare module 'vue-router/auto-routes' { routes: '/test-[a-id]' views: never } + 'src/pages/test-broken-syntax.vue': { + routes: '/test-broken-syntax' + views: never + } 'src/pages/todos/index.vue': { routes: '/todos/' views: never diff --git a/src/core/definePage.spec.ts b/src/core/definePage.spec.ts index 07d92d4f8..f1d4722b0 100644 --- a/src/core/definePage.spec.ts +++ b/src/core/definePage.spec.ts @@ -153,7 +153,7 @@ definePage({ expect(result?.code).toMatchSnapshot() }) - it('throws if definePage uses a variable from the setup', async () => { + it('handles definePage using a variable from setup gracefully', async () => { const code = ` ` - // the function syntax works with sync and async errors - await expect(async () => { - await definePageTransform({ - code, - id: 'src/pages/basic.vue&definePage&vue', - }) - }).rejects.toThrowError() + const result = await definePageTransform({ + code, + id: 'src/pages/basic.vue&definePage&vue', + }) + + // Should return empty object instead of throwing + expect(result).toBe('export default {}') }) it('extracts name and path', async () => { @@ -254,4 +254,128 @@ export default { path: '/custom', }) }) + + describe('error handling', () => { + it('handles syntax errors gracefully when extracting definePage', async () => { + const codeWithSyntaxError = ` + + + + ` + + const result = await definePageTransform({ + code: codeWithSyntaxError, + id: 'src/pages/broken.vue?definePage&vue', + }) + + // Should return empty object instead of crashing + expect(result).toBe('export default {}') + }) + + it('handles syntax errors gracefully when removing definePage from source', async () => { + const codeWithSyntaxError = ` + + + + ` + + const result = await definePageTransform({ + code: codeWithSyntaxError, + id: 'src/pages/broken.vue', + }) + + // Should return undefined (no transform) instead of crashing + expect(result).toBeUndefined() + }) + + it('handles malformed definePage object gracefully', async () => { + const codeWithMalformedObject = ` + + ` + + const result = await definePageTransform({ + code: codeWithMalformedObject, + id: 'src/pages/malformed.vue?definePage&vue', + }) + + expect(result).toBe('export default {}') + }) + + it('handles completely invalid JavaScript syntax gracefully', async () => { + const codeWithInvalidSyntax = ` + + ` + + const result = await definePageTransform({ + code: codeWithInvalidSyntax, + id: 'src/pages/invalid.vue?definePage&vue', + }) + + expect(result).toBe('export default {}') + }) + + it('handles extractDefinePageNameAndPath with syntax errors gracefully', async () => { + const codeWithSyntaxError = ` + + ` + + const result = await extractDefinePageNameAndPath( + codeWithSyntaxError, + 'src/pages/broken.vue' + ) + + // Should return null/undefined instead of crashing + expect(result).toBeUndefined() + }) + + it('handles unclosed brackets in definePage gracefully', async () => { + const codeWithUnclosedBracket = ` + + ` + + const result = await definePageTransform({ + code: codeWithUnclosedBracket, + id: 'src/pages/unclosed.vue?definePage&vue', + }) + + expect(result).toBe('export default {}') + }) + }) }) diff --git a/src/core/definePage.ts b/src/core/definePage.ts index ca420eb1e..89e18cbb3 100644 --- a/src/core/definePage.ts +++ b/src/core/definePage.ts @@ -35,17 +35,24 @@ function getCodeAst(code: string, id: string) { let offset = 0 let ast: Program | undefined const lang = getLang(id.split(MACRO_DEFINE_PAGE_QUERY)[0]!) - if (lang === 'vue') { - const sfc = parseSFC(code, id) - if (sfc.scriptSetup) { - ast = sfc.getSetupAst() - offset = sfc.scriptSetup.loc.start.offset - } else if (sfc.script) { - ast = sfc.getScriptAst() - offset = sfc.script.loc.start.offset + + try { + if (lang === 'vue') { + const sfc = parseSFC(code, id) + if (sfc.scriptSetup) { + ast = sfc.getSetupAst() + offset = sfc.scriptSetup.loc.start.offset + } else if (sfc.script) { + ast = sfc.getScriptAst() + offset = sfc.script.loc.start.offset + } + } else if (/[jt]sx?$/.test(lang)) { + ast = babelParse(code, lang) } - } else if (/[jt]sx?$/.test(lang)) { - ast = babelParse(code, lang) + } catch (error) { + // If there's a syntax error in parsing, we can't extract definePage + // Return undefined AST to indicate parsing failure + ast = undefined } const definePageNodes: CallExpression[] = (ast?.body || []) @@ -77,112 +84,123 @@ export function definePageTransform({ return isExtractingDefinePage ? 'export default {}' : undefined } - const { ast, offset, definePageNodes } = getCodeAst(code, id) - if (!ast) return - - if (!definePageNodes.length) { - return isExtractingDefinePage - ? // e.g. index.vue?definePage that contains a commented `definePage() - 'export default {}' - : // e.g. index.vue that contains a commented `definePage() - null - } else if (definePageNodes.length > 1) { - throw new SyntaxError(`duplicate definePage() call`) - } - - const definePageNode = definePageNodes[0]! - - // we only want the page info - if (isExtractingDefinePage) { - const s = new MagicString(code) - // remove everything except the page info - - const routeRecord = definePageNode.arguments[0] - - if (!routeRecord) { - throw new SyntaxError( - `[${id}]: definePage() expects an object expression as its only argument` - ) + try { + const { ast, offset, definePageNodes } = getCodeAst(code, id) + if (!ast) return isExtractingDefinePage ? 'export default {}' : undefined + + if (!definePageNodes.length) { + return isExtractingDefinePage + ? // e.g. index.vue?definePage that contains a commented `definePage() + 'export default {}' + : // e.g. index.vue that contains a commented `definePage() + null + } else if (definePageNodes.length > 1) { + warn(`duplicate definePage() call in ${id}`) + return isExtractingDefinePage ? 'export default {}' : undefined } - const scriptBindings = ast.body ? getIdentifiers(ast.body) : [] + const definePageNode = definePageNodes[0]! - // this will throw if a property from the script setup is used in definePage - checkInvalidScopeReference(routeRecord, MACRO_DEFINE_PAGE, scriptBindings) + // we only want the page info + if (isExtractingDefinePage) { + const s = new MagicString(code) + // remove everything except the page info - s.remove(offset + routeRecord.end!, code.length) - s.remove(0, offset + routeRecord.start!) - s.prepend(`export default `) + const routeRecord = definePageNode.arguments[0] - // find all static imports and filter out the ones that are not used - const staticImports = findStaticImports(code) + if (!routeRecord) { + warn(`[${id}]: definePage() expects an object expression as its only argument`) + return 'export default {}' + } - const usedIds = new Set() - const localIds = new Set() + const scriptBindings = ast.body ? getIdentifiers(ast.body) : [] - walkAST(routeRecord, { - enter(node) { - // skip literal keys from object properties - if ( - this.parent?.type === 'ObjectProperty' && - this.parent.key === node && - // still track computed keys [a + b]: 1 - !this.parent.computed && - node.type === 'Identifier' - ) { - this.skip() - } else if ( - // filter out things like 'log' in console.log - this.parent?.type === 'MemberExpression' && - this.parent.property === node && - !this.parent.computed && - node.type === 'Identifier' - ) { - this.skip() - // types are stripped off so we can skip them - } else if (node.type === 'TSTypeAnnotation') { - this.skip() - // track everything else - } else if (node.type === 'Identifier' && !localIds.has(node.name)) { - usedIds.add(node.name) - // track local ids that could shadow an import - } else if ('scopeIds' in node && node.scopeIds instanceof Set) { - // avoid adding them to the usedIds list - for (const id of node.scopeIds as Set) { - localIds.add(id) + // this will throw if a property from the script setup is used in definePage + try { + checkInvalidScopeReference(routeRecord, MACRO_DEFINE_PAGE, scriptBindings) + } catch (error) { + warn(`[${id}]: ${error instanceof Error ? error.message : 'Invalid scope reference in definePage'}`) + return 'export default {}' + } + + s.remove(offset + routeRecord.end!, code.length) + s.remove(0, offset + routeRecord.start!) + s.prepend(`export default `) + + // find all static imports and filter out the ones that are not used + const staticImports = findStaticImports(code) + + const usedIds = new Set() + const localIds = new Set() + + walkAST(routeRecord, { + enter(node) { + // skip literal keys from object properties + if ( + this.parent?.type === 'ObjectProperty' && + this.parent.key === node && + // still track computed keys [a + b]: 1 + !this.parent.computed && + node.type === 'Identifier' + ) { + this.skip() + } else if ( + // filter out things like 'log' in console.log + this.parent?.type === 'MemberExpression' && + this.parent.property === node && + !this.parent.computed && + node.type === 'Identifier' + ) { + this.skip() + // types are stripped off so we can skip them + } else if (node.type === 'TSTypeAnnotation') { + this.skip() + // track everything else + } else if (node.type === 'Identifier' && !localIds.has(node.name)) { + usedIds.add(node.name) + // track local ids that could shadow an import + } else if ('scopeIds' in node && node.scopeIds instanceof Set) { + // avoid adding them to the usedIds list + for (const id of node.scopeIds as Set) { + localIds.add(id) + } } - } - }, - leave(node) { - if ('scopeIds' in node && node.scopeIds instanceof Set) { - // clear out local ids - for (const id of node.scopeIds as Set) { - localIds.delete(id) + }, + leave(node) { + if ('scopeIds' in node && node.scopeIds instanceof Set) { + // clear out local ids + for (const id of node.scopeIds as Set) { + localIds.delete(id) + } } + }, + }) + + for (const imp of staticImports) { + const importCode = generateFilteredImportStatement( + parseStaticImport(imp), + usedIds + ) + if (importCode) { + s.prepend(importCode + '\n') } - }, - }) - - for (const imp of staticImports) { - const importCode = generateFilteredImportStatement( - parseStaticImport(imp), - usedIds - ) - if (importCode) { - s.prepend(importCode + '\n') } - } - return generateTransform(s, id) - } else { - // console.log('!!!', definePageNode) + return generateTransform(s, id) + } else { + // console.log('!!!', definePageNode) - const s = new MagicString(code) + const s = new MagicString(code) - // s.removeNode(definePageNode, { offset }) - s.remove(offset + definePageNode.start!, offset + definePageNode.end!) + // s.removeNode(definePageNode, { offset }) + s.remove(offset + definePageNode.start!, offset + definePageNode.end!) - return generateTransform(s, id) + return generateTransform(s, id) + } + } catch (error) { + // Handle any syntax errors or parsing errors gracefully + warn(`[${id}]: Failed to process definePage: ${error instanceof Error ? error.message : 'Unknown error'}`) + return isExtractingDefinePage ? 'export default {}' : undefined } } @@ -192,57 +210,62 @@ export function extractDefinePageNameAndPath( ): { name?: string | false; path?: string } | null | undefined { if (!sfcCode.includes(MACRO_DEFINE_PAGE)) return - const { ast, definePageNodes } = getCodeAst(sfcCode, id) - if (!ast) return + try { + const { ast, definePageNodes } = getCodeAst(sfcCode, id) + if (!ast) return - if (!definePageNodes.length) { - return - } else if (definePageNodes.length > 1) { - throw new SyntaxError(`duplicate definePage() call`) - } + if (!definePageNodes.length) { + return + } else if (definePageNodes.length > 1) { + warn(`duplicate definePage() call in ${id}`) + return + } - const definePageNode = definePageNodes[0]! + const definePageNode = definePageNodes[0]! - const routeRecord = definePageNode.arguments[0] - if (!routeRecord) { - throw new SyntaxError( - `[${id}]: definePage() expects an object expression as its only argument` - ) - } + const routeRecord = definePageNode.arguments[0] + if (!routeRecord) { + warn(`[${id}]: definePage() expects an object expression as its only argument`) + return + } - if (routeRecord.type !== 'ObjectExpression') { - throw new SyntaxError( - `[${id}]: definePage() expects an object expression as its only argument` - ) - } + if (routeRecord.type !== 'ObjectExpression') { + warn(`[${id}]: definePage() expects an object expression as its only argument`) + return + } - const routeInfo: Pick = {} - - for (const prop of routeRecord.properties) { - if (prop.type === 'ObjectProperty' && prop.key.type === 'Identifier') { - if (prop.key.name === 'name') { - if ( - prop.value.type !== 'StringLiteral' && - (prop.value.type !== 'BooleanLiteral' || prop.value.value !== false) - ) { - warn( - `route name must be a string literal or false. Found in "${id}".` - ) - } else { - // TODO: why does TS not narrow down the type? - routeInfo.name = prop.value.value as string | false - } - } else if (prop.key.name === 'path') { - if (prop.value.type !== 'StringLiteral') { - warn(`route path must be a string literal. Found in "${id}".`) - } else { - routeInfo.path = prop.value.value + const routeInfo: Pick = {} + + for (const prop of routeRecord.properties) { + if (prop.type === 'ObjectProperty' && prop.key.type === 'Identifier') { + if (prop.key.name === 'name') { + if ( + prop.value.type !== 'StringLiteral' && + (prop.value.type !== 'BooleanLiteral' || prop.value.value !== false) + ) { + warn( + `route name must be a string literal or false. Found in "${id}".` + ) + } else { + // TODO: why does TS not narrow down the type? + routeInfo.name = prop.value.value as string | false + } + } else if (prop.key.name === 'path') { + if (prop.value.type !== 'StringLiteral') { + warn(`route path must be a string literal. Found in "${id}".`) + } else { + routeInfo.path = prop.value.value + } } } } - } - return routeInfo + return routeInfo + } catch (error) { + // Handle any syntax errors or parsing errors gracefully + warn(`[${id}]: Failed to extract definePage info: ${error instanceof Error ? error.message : 'Unknown error'}`) + return + } } // TODO: use