-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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: ignore ENOENT in injectSourcesContent
#2904
fix: ignore ENOENT in injectSourcesContent
#2904
Conversation
Would it be possible to differentiate between malformed source maps and virtual modules so we can emit a warning when the maps are malformed instead of a silent ignore? Also, do you think it is possible to add a test case? |
Is this |
Maybe, should we bring this up with Evan? |
Am I correct that with the changes to this PR it now throws errors once again in the "malformed sourcemap" case, as it currently does? I think it would be very helpful if malformed source maps in dependencies, whether from bad paths or just failure to package the source with the NPM package, didn't completely kill the build. This is an especially sensitive case IMO because it mostly comes up with dependencies where there's a much longer turnaround to get the source maps fixed. |
@dimfeld That's a good point. It's not a fatal error, so we shouldn't kill the build. 👍 |
This is important for virtual modules and malformed source maps, both of which can throw ENOENT errors when injecting `sourcesContent`.
Also, when the `file` is a virtual module, assume source paths are already absolute.
d169c1a
to
ed09cb7
Compare
@patak-js @antfu This PR has been reworked and rebased. |
Hi @patak-js, I wonder if this PR could be released to a new version of Vite soon? Thanks! |
We are doing one release per week, it will be out next tuesday |
Hi @aleclarson and @patak-js , vite/packages/vite/src/node/server/transformRequest.ts Lines 131 to 152 in 828a8e5
The transform will call vite/packages/vite/src/node/utils.ts Line 434 in 828a8e5
remapping with excludeContent option as true .
|
@wmzy do you see an issue then with the way this PR is implemented? Could you create a bug report if that is the case? |
@patak-js this PR fixed the bug in I will try a PR to optimize this. It's hard to explain with my poor English. |
@patak-js The PR has been created, could you review it? |
Description
This is important for virtual modules and malformed source maps, both of which can throw
ENOENT
errors when injectingsourcesContent
into source maps.Solution
Expect sourcemaps for virtual modules to either contain a pre-populated
sourcesContent
array or have asources
array of absolute paths only. Otherwise, a warning will be printed (at most one time).Expect sourcemaps for normal modules to either contain a pre-populated
sourcesContent
array or have asources
array that never points to non-existent files. Relative source paths are allowed, and they're resolved withsourceRoot
(if defined) or the parent directory of the module. When a non-existent source is found, a warning will printed (at most once per sourcemap for the lifetime of the Vite server).What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).