aurelia / webpack-plugin

A plugin for webpack that enables bundling Aurelia applications.
MIT License
90 stars 36 forks source link

Does not respect DllReferencePlugin, leading to oversized bundles #109

Open SteveSandersonMS opened 7 years ago

SteveSandersonMS commented 7 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior:

When using aurelia-webpack-plugin in a Webpack setup that includes DLL references, aurelia-webpack-plugin disregards the DLL references and bundles its own copy of all Aurelia modules instead of taking the modules from the DLL bundle. This means the resulting bundle contains duplicate copies of the Aurelia modules and is much bigger than necessary.

REPRO:

Should respect DllReferencePlugin and therefore not duplicate the Aurelia modules.

Producing reasonably-sized production builds, while retaining fast dev-time builds.

jods4 commented 7 years ago

I feel honored to receive a bug report from @SteveSandersonMS! I have been a long time user of Knockout and it's a great framework, even to this day! You were a precursor in this area :)

Webpack DllPlugin is definitely something I support and that works. The exact config required is not complicated and described here: https://github.com/aurelia/webpack-plugin/wiki/Using-Webpack-DLL I have a demo setup that works here: https://github.com/jods4/aurelia-webpack-build/tree/master/demos/05-DllReferencePlugin

I see you've tried to use dotnet new aurelia CLI. My guess is that the template might not be perfectly set up for for DllReferencePlugin.

Unless there is a problem with the plugin itself, in that case it might be better to open an issue in the cli repo. CC @JeroenVinke @niieani ?

MeirionHughes commented 7 years ago

I wonder if the duplication is because the DLL is bundling the commonjs dist while AureliaPlugin is picking out the es2015 ones?

From the looks of things the DLL setup is largely the same as the other templates. However, the Angular one specifies the source type: https://github.com/aspnet/JavaScriptServices/blob/dev/templates/AngularSpa/webpack.config.js#L58

jods4 commented 7 years ago

@MeirionHughes I'm not all too familiar with the CLI templates but the DLL should be built with AureliaPlugin as well, at least if you're trying to bundle libs that will be loaded with aurelia-loader (otherwise it doesn't matter).

When AureliaPlugin is active, by default native-modules is the preferred distribution when it exists. This is because using ES import/export leads to better Webpack optimizations (exports renaming, tree shaking) but is configurable with dist option.

If you were to build a DLL without AureliaPlugin or with a different dist than when building your app, then yes duplication will occur. Because jQuery/dist/commonjs/jQuery.js is not the same module as jQuery/dist/native-modules/jQuery.js. I have not actually tested this but I'm pretty sure that's how it works.

If you want to confirm your suspicion, it's fairly easy to look at the DLL manifest and compare with webpack --display-modules to sort out if the shared libs are resolved to different files.

dkent600 commented 7 years ago

Yes, I found independently that the issue relates to the module types and came here to report it.

Note I am also using dotnet new Aurelia which is the tool that @EisenbergEffect promoted at the MS Build conference.

To test the hypothesis, I changed this line in webpack.config.js:

new AureliaPlugin({ aureliaApp: 'boot' })

to this:

new AureliaPlugin({ aureliaApp: 'boot', dist: 'commonjs' })

and bingo the dups disappeared.

However, after doing that, when the app runs it no longer enters into the aurelia-bootstrapper (and a bunch of "hot" related stuff seems to have appeared in the bundle that wasn't there before).

So, to fix that, I added this to webpack.config.vendor.js:

new AureliaPlugin({ aureliaApp: undefined, })

And now things work. However, a better solution would be to modify the vendor bundle to use native-modules rather than commonjs, as recommended. (Note I don't see how to do that in the referenced documentation).

To achieve that I reverted webpack.config.js to this:

new AureliaPlugin({ aureliaApp: 'boot' })

and changed this in webpack.config.vendor.js:

new AureliaPlugin({ aureliaApp: undefined, dist: 'native-modules'

And voila, everything is native-modules.

So there may be a documentation issue here, as well as an issue with dotnet new Aurelia (JavaScriptServices?)

I wonder to whom this should be reported in the realm of the CLI?

arjendeblok commented 7 years ago

I had the same issue. Including the AureliaPlugin in the vendor Webpack config did the 'trick'.

I wonder how you can force webpack to use a specific module format when specifying modules in a configuration file. I cannot find anything in the WebPack documentation.

jods4 commented 7 years ago

Two notes that are pointed out in the wiki about DLL:

  1. As of RC2 you need to add option aureliaApp: undefined in the DLL config. I'll check if I can do that automatically in the plugin, I'm wondering why I did not...
  2. AureliaPlugin absolutely has to be present in the DLL config as well.

If the templates generated by Aurelia CLI don't follow these patterns they should be updated.

Given 2., I'm surprised libraries do not resolve to the same distribution?

dkent600 commented 7 years ago

@jods4

Sorry my first comment wasn't very good.

The bottom line is that the DLL (vendor) bundle config is missing this: new AureliaPlugin({ aureliaApp: undefined}). Adding that solves the problem. The app bundle was resolving to the AureliaPlugin default of native-modules, but lacking the AureliaPlugin, the DLL config was resolving to webpack's default of commonjs.

@arjendeblok you can add dist: '[module type name]' to your AureliaPlugin config to change the module type. The default appears to be native-modules. (Would be nice if this were documented.)

jods4 commented 7 years ago

OK so it seems the problem here is that the CLI template for DLL config is incomplete. @JeroenVinke @EisenbergEffect can you change the CLI template? It's an easy fix.

@dkent600 The default is in the documentation: https://github.com/aurelia/webpack-plugin/wiki/AureliaPlugin-options#dist It even says why this is the default. You'll have best results with Webpack when using ES import/exports (tree shaking, exports renaming, and with 3.0 scope hoisting).

PS: There are other under-the-hood reasons why including AureliaPlugin is mandatory if any module in your DLL are related to Aurelia.

dkent600 commented 7 years ago

@jods4 Sorry, I was looking here, which is where you referred us to earlier in this thread. Thanks for the additional link.

jods4 commented 7 years ago

@dkent600 the whole wiki is a good read if you're using Aurelia + Webpack ;)

jods4 commented 7 years ago

FYI I have altered the behavior in RC3. When a DllPlugin is detected, there will be no default value for aureliaApp, which leads to the more intuitive usage of just new AureliaPlugin(), whether you're building a DLL or not.

Using AureliaPlugin is still mandatory in Aurelia-related DLLs and the CLI templates have to be adjusted anyway.