Skip to content
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): log esbuild error when scanning deps #11977

Merged
merged 3 commits into from
Feb 22, 2023

Conversation

fi3ework
Copy link
Member

@fi3ework fi3ework commented Feb 8, 2023

Description

from #11966
The error message thrown from esbuild in the scanning stage now is quite opaque. This PR added essential error messages and stack traces.

log before:

image

log now:

image

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@bluwy bluwy added p2-nice-to-have Not breaking anything but nice to have (priority) feat: deps optimizer Esbuild Dependencies Optimization labels Feb 18, 2023
@bluwy
Copy link
Member

bluwy commented Feb 20, 2023

I tested this locally but it seemed the message is still a bit intertwined, since it logs and errors at the same time. I went in and updated the log error 😬 It should look like this now:

image

Let me know if this works for you. There's two new lines under esbuild frames, but I'm leaving it as-is to not mess with esbuild's formatted messages in case it changes.

@fi3ework
Copy link
Member Author

The only thing I am concerned about is the stack is not completed. It lost the stack sections in the image below which I added by composing a new Error() which may result in the log being a bit ugly. 😬
image

@bluwy
Copy link
Member

bluwy commented Feb 20, 2023

I removed that as I'm not sure if it's important. We need to have the esbuild stacktrace too so they know it's coming from esbuild. Between the two frames, I think keeping esbuild's one is enough. The user can search for "Failed to scan dependencies from entries" in Vite's code if they want to know where this error is emitted.

@fi3ework
Copy link
Member Author

LGTM, nice polishing.

@bluwy bluwy added this to the 4.2 milestone Feb 21, 2023
@bluwy bluwy changed the title fix: log esbuild error when scanning deps fix(optimizer): log esbuild error when scanning deps Feb 22, 2023
@bluwy bluwy merged commit 20e6060 into vitejs:main Feb 22, 2023
@fi3ework fi3ework deleted the log-esbuild-error branch February 22, 2023 09:52
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
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 p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants