-
Notifications
You must be signed in to change notification settings - Fork 86
Conversation
Hey @arantes555 - this looks like a good PR. My question is, does Also this PR has some conflicts - so if you think we should really add this it would be great to get it rebased 😃 |
Hi @keithamus ! Thanks for checking this out 😄 Nope, the Indeed, there are some merge conflicts, which is to be expected as the PR is more than a year old 😜 . |
b6dcc5d
to
4387b8a
Compare
@keithamus : branch has been rebased, and some cleanup has been done :) I believe it's good to merge on my end! |
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.
Im happy with this - but I'll ask @lukastaegert for a second 👍 before we go ahead and merge.
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.
There seems to be some overlap with rollup's own "externals" function but I still see some use-cases for this feature. One minor performance suggestion, otherwise 👍
src/index.js
Outdated
@@ -99,6 +100,8 @@ export default function nodeResolve ( options = {} ) { | |||
id = resolve( importer, '..', importee ); | |||
} | |||
|
|||
if (only && !only.some(pattern => id.match(pattern))) return null; |
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.
For performance reasons it might also make sense to initially convert all entries to RegExp (in L.47 where it will only happen once) and then use pattern.test(id)
instead (which will make all invocations faster, cf. e.g. https://stackoverflow.com/a/10940138)
@lukastaegert : I just pushed a minor edit with your suggestion ! |
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.
Nice!
|
Hello,
However, for my project, I needed the ability to limit the resolution of node_modules to only modules matching a pattern, so I added this option. The implementation is very straightforward, using
String.prototype.match
.Using it is very simple. It works with both strings and regexes: