Mark dependencies as webpack externals#679
Conversation
Signed-off-by: Julius Härtl <[email protected]>
juliusknorr
requested review from
ChristophWurst,
korelstar,
marcoambrosini and
skjnldsv
|
Nice! Why not marking |
But then they will be considered as extraneous dependencies, no? |
|
No, as they are still part of the dependencies in package.json:
And for the app itself only exports of @nextcloud/vue should be imported so the linting rule should still be fine. |
|
I gave this a test with Mail. The bundle size is a sum of all .js files, without map files. Built against nc-vue master: 2692B This means the new bundle will be just 90.79% of the size. This is quite impressive considering that Mail uses many of the components of this lib. |
skjnldsv
added
configuration
dependencies
ChristophWurst
left a comment
ChristophWurst
left a comment
There was a problem hiding this comment.
Does what it promises to do :)
webpack.common.js
Outdated
| root: 'Vue' | ||
| } | ||
| }, | ||
| '@nextcloud/axios': '@nextcloud/axios', |
There was a problem hiding this comment.
Can't we automate this somehow?
Instead of manually specifying them?
There was a problem hiding this comment.
I pushed another commit to exclude all from node_modules. See https://www.npmjs.com/package/webpack-node-externals#why-not-just-use-a-regex-in-the-webpack-config for why the additional package. 😉
Seems to still work fine :)
There was a problem hiding this comment.
Signed-off-by: Julius Härtl <[email protected]>
4 participants
This PR adds the external dependencies we use to the webpack externals list, so they are not included in the bundle. Since they are part of the dependency section they will still be installed by npm when app developers install our package and webpack will take care of resolving them properly then.
Before
After
The size of the saving in the app bundles of course depends on which components are used, but when using multiple components that include the usage of v-tooltip, popper.js will only be included once with this PR.
A quick test run on the deck vue branch saved me ~300kb in duplicate dependencies.