Skip to content

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented May 21, 2025

Description

When this computeEntries call errored, an error was happening during the error handling and was hiding the actual error.

const esbuildContext: Promise<BuildContext | undefined> = computeEntries(
environment,
).then((computedEntries) => {

TypeError: Cannot read properties of undefined (reading 'join')
    at file:///D:/documents/GitHub/vite/packages/vite/dist/node/chunks/dep-BXs3190j.js:10726:13
    at file:///D:/documents/GitHub/vite/packages/vite/dist/node/chunks/dep-BXs3190j.js:34673:20

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)

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) feat: deps optimizer Esbuild Dependencies Optimization labels May 21, 2025
const prependMessage = colors.red(`\
const prependMessage = colors.red(`\
Failed to scan for dependencies from entries:
${entries.join('\n')}
Copy link
Member Author

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.

Comment on lines +210 to +219
} catch (e) {
environment.logger.error(
colors.red(
'(!) Failed to run dependency scan. ' +
'Skipping dependency pre-bundling. ' +
e.stack,
),
)
return
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part of the change just adds some context to the error. It looks like
image

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: {} }
Copy link
Member

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

Copy link
Member Author

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.

@patak-dev patak-dev merged commit b742b46 into vitejs:main May 22, 2025
15 checks passed
@sapphi-red sapphi-red deleted the fix/show-error-when-compute-entries-failed branch May 23, 2025 02:33
moonlitusun pushed a commit to moonlitusun/vite that referenced this pull request May 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: deps optimizer Esbuild Dependencies Optimization p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants