-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
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(scan): correctly resolve virtual modules #5711
Conversation
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 change is fine with me as long as we don't reintroduce the html:virtual-module:
bug (https://discord.com/channels/804011606160703521/908717851827904532/908719798433771540)
I'm assuming you tested that though? Do all the vite-plugin-svelte
tests pass with this change?
Yeah' I've tested this locally on vite-plugin-svelte's |
To be safe, I can try to get my setup working tomorrow and run the tests again. We may still have time during beta. |
Hmm. When I run the tests it looks like we go from 3 tests failing before this PR to 73 tests failing after this PR. A lot of the failing tests are in the |
There's currently two kinds of tests in |
I was (finally) able to get tests running locally. Yeah looks like there's still some things not caught, putting this as draft for now. |
I made the real fix now, and it was because in esbuild, we had to resolve the virtual module to an actual file path for its file imports to be resolved, which was causing the initial |
Description
Reverts #5659 (cc @benmccann)
Resolve the virtual module to an actual file path for its file imports to be resolved by esbuild. Otherwise the
importer
would bevirtual-module:path/to/file
which fails to resolve.Additional context
Interfering with all Svelte and Vue projects as dependencies aren't properly crawled on first run.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).