Support using packages in imports resolved by the glob resolver #8097
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
devongovett
reviewed
May 17, 2022
|
|
||
| module.exports = async function () { | ||
| await promise.all([scoped, unscoped]); | ||
| return scoped.a + scoped.b + unscoped.x + unscoped.y; |
There was a problem hiding this comment.
I think it should be Promise.all? And each key on the returned object from import() is a function that also resolves to a promise, so this should be await scoped.a() + await scoped.b() ...
| name: 'index.js', | ||
| assets: ['*.js', '*.js', 'a.js', 'b.js', 'x.js', 'y.js', 'index.js'], | ||
| }, | ||
| ]); |
There was a problem hiding this comment.
Probably should run the resulting bundle here as in the other tests to ensure it actually works
| // if the specifier does not start with /, ~, or . then it's not a path but package-ish - we resolve | ||
| // the package first, and then append the rest of the path | ||
| if (!/^[/~.]/.test(specifier)) { | ||
| specifier = path.normalize(specifier); |
There was a problem hiding this comment.
I think globs are supposed to always use / as a separator, even on Windows. See https://github.com/micromatch/micromatch#backslashes. So this might not be needed.
marcins
force-pushed
the
mszczepanski/glob-node-resolver
branch
from
925fec2
to
a6502dc
Compare
devongovett
approved these changes
May 23, 2022
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
↪️ Pull Request
Implements the ability for the glob resolver to resolve globs in packages, fixes #7945.
💻 Examples
With this change the glob resolver will work with a glob like
import('@scope/pkg/i18n/*.js'), by first resolving@scope/pkgto a path, and then appending/i18n/*.js.🚨 Test instructions
An integration test has been added for both a regular
requireas well as an asyncimport. Please provide feedback if the tests added are not comprehensive enough.✔️ PR Todo
unitintegration tests for this change