-
Notifications
You must be signed in to change notification settings - Fork 137
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
Rename #embroider_compat/* to @embroider/virtual/* #2026
Comments
is this because otherwise it wont pass the bare import filter in vite internal dep scan hook? |
Yes 👍 |
Okay, just note that there are multiple ways to refer to the same file
For each esbuild would generate a separate bundle id. I'm not entirely sure if it also generates different chunks or just generates another file which imports the same chunks. But it would be less efficient anyway. So. I'm not sure how much this would help in the end. Short term to get more scan through esbuild. It looks nicer at least :) Furthermore, when adding new import during dev, vite additionally checks if the import is resolvable with default node resolve and only then makes a new dep scan on it. We would need to alias all of those to one same import specifier and make it node resolve able. |
This is not going to be necessary. The work in #2029 demonstrates that we can do full resolution in our esbuild plugin before vite's depscan sees anything. |
Rename
#embroider_compat/(component|helper|modifier|ambiguous)/<path>
for invokable to@embroider/virtual/(component|helper|modifier|ambiguous)/<path>
to stop confusing esbuild.#embroider_compat
.shared-internals/src/paths.ts
: pay attention toconst postfixRE = /[?#].*$/s;
that used to match#embroider_compat
pattern: if we no longer have any request from a file starting with#
, then there's no reason to separatepostfixRE
andpostfixREQueryParams
cases.cleanUrl('@embroider/virtual/fancy?query="p"', false)
returns@embroider/virtual/fancy
and we no longer need a special pattern to have a#embroider
considered part of the path name.The text was updated successfully, but these errors were encountered: