-
-
Notifications
You must be signed in to change notification settings - Fork 105
Fix: Gracefully handle syntax errors in definePage to prevent Vite server crashes #692
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/experimental-router
Are you sure you want to change the base?
Changes from 2 commits
09c84cd
1923444
dded05e
5992675
d8052e0
f41937a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 = ` | ||
<script setup> | ||
const a = 1 | ||
|
@@ -162,13 +162,13 @@ definePage({ | |
}) | ||
</script> | ||
` | ||
// 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 = ` | ||
<script setup> | ||
definePage({ | ||
name: 'test',, // syntax error: extra comma | ||
path: '/test' | ||
}) | ||
</script> | ||
|
||
<template> | ||
<div>hello</div> | ||
</template> | ||
` | ||
|
||
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 {}') | ||
posva marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
|
||
it('handles syntax errors gracefully when removing definePage from source', async () => { | ||
const codeWithSyntaxError = ` | ||
<script setup> | ||
const a = 1 | ||
definePage({ | ||
name: 'test',, // syntax error: extra comma | ||
path: '/test' | ||
}) | ||
const b = 1 | ||
</script> | ||
|
||
<template> | ||
<div>hello</div> | ||
</template> | ||
` | ||
|
||
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 () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is redundant, syntax errors are already tested. Ensure you don't have duplicated testss There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the redundant "handles malformed definePage object gracefully" test as it was duplicating syntax error testing already covered by other tests. (5992675) |
||
const codeWithMalformedObject = ` | ||
<script setup> | ||
definePage({ | ||
name: 'test' | ||
path: '/test' // missing comma | ||
}) | ||
</script> | ||
` | ||
|
||
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 () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, remove it, it's just syntax error There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the redundant "handles completely invalid JavaScript syntax gracefully" test as it was just another syntax error variation already covered by existing tests. (5992675) |
||
const codeWithInvalidSyntax = ` | ||
<script setup> | ||
definePage({ | ||
name: 'test', | ||
path: '/test' | ||
invalid javascript syntax here ### | ||
}) | ||
</script> | ||
` | ||
|
||
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 = ` | ||
<script setup> | ||
definePage({ | ||
name: 'test',, // syntax error: extra comma | ||
path: '/test' | ||
}) | ||
</script> | ||
` | ||
|
||
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 () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is just a syntax error, it's a redundant test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the redundant "handles unclosed brackets in definePage gracefully" test as it was just another syntax error variation already covered by existing tests. (d8052e0) |
||
const codeWithUnclosedBracket = ` | ||
<script setup> | ||
definePage({ | ||
name: 'test', | ||
path: '/test' | ||
// missing closing bracket | ||
</script> | ||
` | ||
|
||
const result = await definePageTransform({ | ||
code: codeWithUnclosedBracket, | ||
id: 'src/pages/unclosed.vue?definePage&vue', | ||
}) | ||
|
||
expect(result).toBe('export default {}') | ||
}) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this variable for a piece of code can be extracted in the outer scope. Share it among tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extracted the codeWithSyntaxError variable to the outer scope of the error handling describe block to share it among the tests, avoiding duplication. (5992675)