-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
fix(optimizer): show error when computeEntries
failed
#20079
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
fix(optimizer): show error when computeEntries
failed
#20079
Conversation
const prependMessage = colors.red(`\ | ||
const prependMessage = colors.red(`\ | ||
Failed to scan for dependencies from entries: | ||
${entries.join('\n')} |
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 is the place the TypeError: Cannot read properties of undefined (reading 'join')
error was happening.
When computeEntries
throws an error, entries = computedEntries
in the then
after computeEntries
is not executed and entries
will be kept as undefined
.
The old code was not clear which variable lives at which part of the code. TypeScript is not good with analyzing code flow that uses promise + then + temp variable.
I refactored the code to use a single async function so that the code flow is a straight line. This should be easier to understand for both us and TypeScript.
} catch (e) { | ||
environment.logger.error( | ||
colors.red( | ||
'(!) Failed to run dependency scan. ' + | ||
'Skipping dependency pre-bundling. ' + | ||
e.stack, | ||
), | ||
) | ||
return | ||
} |
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.
if (e.errors && e.message.includes('The build was canceled')) { | ||
// esbuild logs an error when cancelling, but this is expected so | ||
// return an empty result instead | ||
return { deps: {}, missing: {} } |
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.
Should this return undefined
like in line 168? Now that there is a fallback at 222, maybe it is good to normalize it. Or if not, return { deps: {}, missing: {} }
everywhere instead of the fallback
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.
That makes sense. I've updated to return undefined
.
Description
When this
computeEntries
call errored, an error was happening during the error handling and was hiding the actual error.vite/packages/vite/src/node/optimizer/scan.ts
Lines 132 to 134 in 61b6b96
reproduction: https://stackblitz.com/edit/vitejs-vite-nte18shp?file=package.json,vite.config.js&terminal=dev
This PR fixes the error handling logic so that it doesn't hide the actual error. This PR does not fix the actual error (I'll do it in a separate PR).
reported at #15834 (reply in thread)