fix: url with non-existent assets in dynamic URLs should fallback to rawUrl (#11157) #11233
Conversation
|
@Jevon617 would you be able to update this PR? It had a merge conflict which I tried to address, but the tests are failing now |
|
Certainly, I will work on addressing the issue and get back to you as soon as possible. |
|
@Jevon617 I just noticed that I lost part of your change while addressing the merge conflict. I fixed that and the PR is passing now. Sorry for any trouble |
|
@benmccann Got it, glad to hear it's working now! |
Co-authored-by: Ben McCann <[email protected]>
| s.update( | ||
| index, | ||
| index + exp.length, | ||
| `new URL( | ||
| import.meta.glob(${JSON.stringify(pattern)}, | ||
| { eager: true, import: 'default', as: 'url' } | ||
| )[${rawUrl}] || \`${builtUrl}\`, self.location)`, | ||
| ) |
There was a problem hiding this comment.
There was a problem hiding this comment.
To me personally, I feel the statement of return new URL(./dir/${name}.png, import.meta.url).href making Vite bundle all png files under that directory is a bit too implicit. So I also consider this is not a good solution. To me, I feel we should resolve it to the correct path like v3.1 but not maybe not bundle all the assets by default.
There was a problem hiding this comment.
In version 3.1, the assetImportMetaUrlPlugin was not introduced in dev mode, so undefined will not appear.
There was a problem hiding this comment.
but in build mode, undefined will appear.
There was a problem hiding this comment.
This is a clever way to support the old behavior. But needing to do this for all cases seems wrong to me. The generated code would not be clean. I think that #11157 may not be an issue, but we need to clarify if needed the docs to add a vite-ignore in this case. @antfu what do you think?
Do you mean adding the logic to handle vite-ignore in assetImportMetaUrlPlugin?
Description
Fix #11157.
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).