Allow any alias to be used for external modules #83
Conversation
Previous to 2.27.0, browserify required external module ids to resolve to a filepath. That's no longer the case, which means that grunt-browserify can now make the aliasing / externaling process much more straightforward. See the updated examples for how to implement.
|
Open for review: any comments @shama, @joeybaker, @bclinkinbeard? (Or anybody else for that matter?) |
|
For reference, the browserify PR that added this functionality: browserify/browserify#473 |
|
I only took a cursory glance but looks good to me. Is the alias just coming from the object key inside shim here? https://github.com/jmreidy/grunt-browserify/blob/5aa8f09115e2598e230aa823c9ee90a548909496/examples/complex/Gruntfile.js Might be worth calling out. Nice work! |
|
@bclinkinbeard yep that's exactly right. I'll make a note of that in the README. |
|
This is sweet! I took a quick look through the task code and it looks good enough to me. Thanks! |
| if (expandedExternals.length > 0) { | ||
| expandedExternals.forEach(function (dest) { | ||
| var externalResolved = path.resolve(dest) | ||
| if (grunt.file.exists(externalResolved)) { |
There was a problem hiding this comment.
I think you'll need a second check here for && grunt.file.isFile(externalResolved)). exists will return true on both directories and files, which can also both be returned from grunt.file.expand.
There was a problem hiding this comment.
Yes, that's how I originally implemented it, but there's currently functionality to make an entire directory external (see external-dir test).
There was a problem hiding this comment.
My bad, I missed that. Nifty.
There was a problem hiding this comment.
After reading the test, maybe: && (grunt.file.isFile(externalResolved) || grunt.file.isFile(path.join(externalResolved, '/index.js'))
There was a problem hiding this comment.
Saw you merged this – awesome! Is the comment above worth a PR, or have a misunderstood again?
| var expandedExternals = grunt.file.expand(external); | ||
| if (expandedExternals.length > 0) { | ||
| expandedExternals.forEach(function (dest) { | ||
| var externalResolved = path.resolve(dest) |
There was a problem hiding this comment.
Missing semicolon (not that they are necessary but just to match your existing code style ;)
|
+1 LGTM. Nice work Justin! |
|
Merged and published in |
Previous to 2.27.0, browserify required external module ids to resolve
to a filepath. That's no longer the case, which means that grunt-browserify
can now make the aliasing / externaling process much more
straightforward. See the updated examples for how to implement.