Fix support for Webpack externals. This fixes #471 #495
Conversation
There was a problem hiding this comment.
Hi @deAtog,
As I said in the other PR, I really prefer this version :)
Could you fix the linting issues? It seems that your IDE sometimes used tabs instead of spaces.
By the way I also recommend you to check if there is an EditorConfig plugin for the one you use since many projects (including Encore) have an .editorconfig files nowadays.
|
@Lyrkan fixed the linting issues. |
| * jquery: 'jQuery', | ||
| * react: 'react' | ||
| * }) | ||
| * const nodeExternals = require('webpack-node-externals'); |
There was a problem hiding this comment.
I'm wondering if that's a good idea to put this specific example. I'm a bit scared that some people will think this is a requirement in order to add externals...
There was a problem hiding this comment.
Feel free to suggest an alternate example, but webpack-node-externals is probably the most commonly used functional usage here. Maybe spitting it into two examples would be better? One that only adds jquery and react, and one that uses webpack-node-externals?
There was a problem hiding this comment.
Hm... maybe splitting it would be better yes.
But maybe I'm also worrying for no real reason here...
@weaverryan What do you think?
There was a problem hiding this comment.
I think splitting is better since I really can't see someone defining 'jquery' and 'react' as externals and then using webpack-node-externals in the same config.
index.js
Outdated
| * react: 'react' | ||
| * }, | ||
| * nodeExternals() | ||
| * ]); | ||
| * | ||
| * @param {object} externals |
There was a problem hiding this comment.
Maybe {*} instead of {object}?
There was a problem hiding this comment.
Yeah, {*} would indeed be better.
| * nodeExternals() | ||
| * // add any valid externals you have | ||
| * nodeExternals(), | ||
| * /^(jquery|\$)$/i |
There was a problem hiding this comment.
That PR is the one that doesn't use rest parameters (as opposed to #472), so you'd need to put these two lines into an array.
|
Thank you @deAtog! |
…d Ellingsworth, Lyrkan, weaverryan) This PR was merged into the master branch. Discussion ---------- Fix support for Webpack externals. This fixes #471 This is an alternative to #472 which does not modify the public Encore API and requires the user to pass an array if they want to add multiple externals at a time. Commits ------- 58dc86d tweaking example c1fbbf3 Split the addExternals example into two separate examples. c0a8d8d Use {*} instead of {object} for externals parameter of addExternals method. 0e012d3 Fix style issues identified by lint. 5ab6822 Include a regex in the tests of addExternals. a51b35f Update documentation for Encore.addExternals 3d6fb55 Ensure a copy of the externals is returned when generating a Webpack config. 84a0dc0 Store externals in an array. This fixes #471
This is an alternative to #472 which does not modify the public Encore API and requires the user to pass an array if they want to add multiple externals at a time.