-
Notifications
You must be signed in to change notification settings - Fork 916
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
Add basic glob support for assets #87
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.
Awesome! I think this makes a lot of sense as the place to handle this logic. Just left 1 small comment about flat().
src/index.ts
Outdated
const globResults = globDependencies | ||
.map(dep => path.join(cwd, 'node_modules', dep)) | ||
.map(dep => glob.sync(dep)) | ||
.flat() |
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.
unfortunately we still need to support Node 8+, and I believe this was introduced in Node 11.
Here's another way you could write this, half getting rid of flat & half just doing some code golf. The nice thing about using a Set is that now that we get deduplication for free (more important now that we have globs). But feel free to use this however you see fit.
const depsList = new Set();
arrayOfDeps.forEach(dep => {
if (!glob.hasMagic(dep)) {
depsList.add(dep);
return;
}
const globLoc = path.join(cwd, 'node_modules', dep);
glob.sync(globLoc).forEach((globMatchLoc) => {
const globMatch = path.relative(`${cwd}/node_modules`, globMatch);
depsList.add(globMatch);
});
});
// Then, just replace the 1-2 later references to arrayOfDeps with the new depsList
@FredKSchott Fair enough. I had written almost exactly what you posted at first, but the array syntax felt much more readable, so I went with that. Forgot |
🚀 This PR has been merged! Once a new release is created, it will become available on npm. Until then, you can load and install it directly from the Pika CDN:
|
Awesome! I'll get a new version cut and released, thanks for tackling! |
This PR adds basic support for glob matching using explicit mode per the conversation in #85. This feature uses the npm package, glob,'s glob-matching syntax.
Example
In package.json
Would yield the output