feat(importMetaGlob): support sub imports pattern#12467
feat(importMetaGlob): support sub imports pattern#12467patak-cat merged 5 commits intovitejs:mainfrom
Conversation
|
|
|
I think we can consider supporting read modules via sub imports path like #12221. There will be 3 glob patterns: normal pattern, alias pattern and sub imports pattern. Each pattern's module entry key has its own style. |
There was a problem hiding this comment.
LGTM. In the future maybe we can improve the custom glob resolver to handle aliases only, but I think this is good enough for now.
EDIT: That might be what you mean in #12467 (comment), but I have no context of that PR and issue yet 😬
|
/ecosystem-ci run |
This comment was marked as outdated.
This comment was marked as outdated.
|
/ecosystem-ci run astro |
This comment was marked as outdated.
This comment was marked as outdated.
|
I am not familiar with astro. Can you help check out why this fails @bluwy Thanks. |
|
I think it's flaky tests happening, but since this is sort-of a feature too, we've pushed it to the next minor so we can test more extensively with Astro. |
|
@bluwy feel free to merge it when you feel it is better, let's start the 4.3 beta period 🙌 |
|
/ecosystem-ci run astro |
This comment was marked as outdated.
This comment was marked as outdated.
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
The current failures also fail in main, except Astro again 😭 I'm quite inclined to merge this still because Astro's CI error doesn't makes sense that it'd be affected by this PR. |
Maybe this pr triggers a bug in astro. It looks like the errors are the same between each ecosystem-run |
|
|
||
| const resolved = normalizePath( | ||
| (await resolveId(glob, importer, { | ||
| custom: { 'glob-imports': isSubImportsPattern }, |
There was a problem hiding this comment.
@bluwy I also think we should merge this one, we have time to fix Astro if it is affecting it (but as you say, I doubt is related)
About the custom option, maybe we should use a namespace for it to avoid collisions? Something like 'vite:import-glob:imports' or 'vite:import-glob-imports' ?
There was a problem hiding this comment.
Yeah if this gets merged, I'll monitor Astro again for errors. A namespace sounds nice too, maybe we should follow the Vite plugin name we use vite:import-glob, since there's also custom['node-resolve'].
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
Updated the (await resolveId(glob, importer, {
custom: { 'vite:import-glob': { isSubImportsPattern } },
})) || glob,Let's move this one to the 4.4 milestone, better to avoid merging just before the release. |
Description
fix #12465
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).