Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: Two AutoDLLPlugin's overwrite each other #30

Open
cgatian opened this issue Jul 6, 2017 · 15 comments
Open

Fixed: Two AutoDLLPlugin's overwrite each other #30

cgatian opened this issue Jul 6, 2017 · 15 comments

Comments

@cgatian
Copy link

cgatian commented Jul 6, 2017

Trying to utilize this plugin and found that when you add two AutoDLLPlugins to the webpack config the last one wins in creating a manifest in .cache.

To reproduce add two plugins, and attempt a build.

@asfktz
Copy link
Owner

asfktz commented Jul 6, 2017

Hi, @cgatian

Thanks for reporting.
I also encountered this bug a few days ago.
It's a top priority, I'll let you know when it's fixed.

@asfktz
Copy link
Owner

asfktz commented Jul 6, 2017

BTW, would you mind sharing what your use case was for more than one instance of the plugin?

@asfktz asfktz added the bug label Jul 6, 2017
@cgatian
Copy link
Author

cgatian commented Jul 6, 2017

@asfktz we were considering migration to this plugin instead of the webpack-dll-bundles-plugin as it hasn't received much attention in the last 6 months,

The setup this webpack starter has basically outlines what we were trying to do. They create two DLL bundles, one for polyfills and a second for vendor. My understanding is, since DLLs should only really be used during development creating two instances really has no benefit. (Please correct me if I'm wrong)

@viankakrisna
Copy link
Contributor

I've anticipated this issue in #13 but does not have a fixture yet. Adding a fixture for this functionality would be a good start so we can test it thoroughly. :)

@asfktz
Copy link
Owner

asfktz commented Jul 7, 2017

@cgatian turns out that using DLL in production is not a bad idea.

I forked your project to better understand your use case. I don't see more than one instance of DllBundlesPlugin in there.

I do see two bundles, polyfills and vendor, but that should be no problem with AutoDllPlugin.

I see that you have separate configs for each environment,
having one instance in each should not be a problem because they run separately.

But it will trigger a rebuild currently. it will be fixed soon.

I plan to fix the multiple instances bug anyway.

I noticed that when I replace DllBundlesPlugin with AutoDllPlugin:

new AutoDllPlugin({
        debug: true,
        inject: true,
        context: helpers.root(),
        filename: '[name]_[hash].js',
        path: './dll',
        entry: {
          polyfills: [
            'core-js',
            'zone.js/dist/zone.js',
            'zone.js/dist/long-stack-trace-zone'
          ],
          vendor: [
            '@angular/platform-browser',
            '@angular/platform-browser-dynamic',
            '@angular/core',
            '@angular/common',
            '@angular/forms',
            '@angular/http',
            '@angular/router',
            '@angularclass/hmr',
            'rxjs',
          ]
        }
 }),

An error is thrown on npm start

....

======================================================================================
 46% building modules 303/306 modules 3 active ...rc/app/+detail/+child-detail/index.ts================================== ng-router-loader ==================================
Importer:    /Users/asafkatz/dev/angular-starter/src/app/+barrel/barrel.routes.ts
Raw Request: ./+child-barrel#ChildBarrelModule
Replacement: function() { return import('/Users/asafkatz/dev/angular-starter/src/app/+barrel/+child-barrel/index')  .then( function(module) { return module['ChildBarrelModule']; } ); }
======================================================================================
2454ms building modules
9ms sealing
0ms optimizing
0ms basic module optimization
4ms module optimization
 75% advanced module optimization
/Users/asafkatz/dev/angular-starter/node_modules/autodll-webpack-plugin/node_modules/webpack/lib/ModuleReason.js:27
			if(!this.module._chunks.has(oldChunk))
                          ^
TypeError: Cannot read property 'has' of undefined
    at ModuleReason.rewriteChunks (/Users/asafkatz/dev/angular-starter/node_modules/autodll-webpack-plugin/node_modules/webpack/lib/ModuleReason.js:27:27)
    at DelegatedModule.rewriteChunkInReasons (/Users/asafkatz/dev/angular-starter/node_modules/autodll-webpack-plugin/node_modules/webpack/lib/Module.js:169:20)
    at Compilation.compilation.plugin (/Users/asafkatz/dev/angular-starter/node_modules/webpack/lib/optimize/RemoveParentModulesPlugin.js:82:15)
    at Compilation.applyPluginsBailResult1 (/Users/asafkatz/dev/angular-starter/node_modules/tapable/lib/Tapable.js:120:27)
    at Compilation.seal (/Users/asafkatz/dev/angular-starter/node_modules/webpack/lib/Compilation.js:574:14)
    at /Users/asafkatz/dev/angular-starter/node_modules/webpack/lib/Compiler.js:493:16
    at /Users/asafkatz/dev/angular-starter/node_modules/tapable/lib/Tapable.js:225:11
    at _addModuleChain (/Users/asafkatz/dev/angular-starter/node_modules/webpack/lib/Compilation.js:481:11)
    at processModuleDependencies.err (/Users/asafkatz/dev/angular-starter/node_modules/webpack/lib/Compilation.js:452:13)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)
EXIT RUNTIME
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] webpack-dev-server: `node --max_old_space_size=4096 node_modules/webpack-dev-server/bin/webpack-dev-server.js "--config" "config/webpack.dev.js" "--open" "--progress" "--profile" "--watch" "--content-base" "src/"`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] webpack-dev-server script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/asafkatz/.npm/_logs/2017-07-07T14_13_04_021Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] server:dev: `npm run webpack-dev-server -- --config config/webpack.dev.js --open --progress --profile --watch --content-base src/`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] server:dev script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/asafkatz/.npm/_logs/2017-07-07T14_13_04_053Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] start: `npm run server:dev`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] start script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/asafkatz/.npm/_logs/2017-07-07T14_13_04_086Z-debug.log

I plan to investigate it.

@cgatian
Copy link
Author

cgatian commented Jul 7, 2017

@asfktz yes they can be used in production in very clever scenarios. However, I believe when you're actually using them in production youre going to want to be very strategic in how the DLLs are built. Something I dont think should be totally automated by a package, but on a case by case basis.

Thanks for taking a look. 👍

@taehwanno
Copy link

taehwanno commented Jul 11, 2017

An error is thrown on npm start

@asfktz I think because autodll-webpack-plugin depends on webpack v3, angular-starterkit using webpack v2 will fail in running npm start. My private project (webpack v2.4.1) also fail in running npm start (execute webpack-dev-server). but by updating to webpackv3, script succeed.

/Users/notaehwan/modusign-client/node_modules/autodll-webpack-plugin/node_modules/webpack/lib/ModuleReason.js:27
			if(!this.module._chunks.has(oldChunk))
			                       ^

TypeError: Cannot read property 'has' of undefined
    at ModuleReason.rewriteChunks (/Users/notaehwan/modusign-client/node_modules/autodll-webpack-plugin/node_modules/webpack/lib/ModuleReason.js:27:27)
    at DelegatedModule.rewriteChunkInReasons (/Users/notaehwan/modusign-client/node_modules/autodll-webpack-plugin/node_modules/webpack/lib/Module.js:169:20)
    at Compilation.compilation.plugin (/Users/notaehwan/modusign-client/node_modules/webpack/lib/optimize/RemoveParentModulesPlugin.js:82:15)
    at Compilation.applyPluginsBailResult1 (/Users/notaehwan/modusign-client/node_modules/tapable/lib/Tapable.js:120:27)
    at Compilation.seal (/Users/notaehwan/modusign-client/node_modules/webpack/lib/Compilation.js:570:14)
    at /Users/notaehwan/modusign-client/node_modules/webpack/lib/Compiler.js:488:16
    at /Users/notaehwan/modusign-client/node_modules/tapable/lib/Tapable.js:225:11
    at _addModuleChain (/Users/notaehwan/modusign-client/node_modules/webpack/lib/Compilation.js:477:11)
    at processModuleDependencies.err (/Users/notaehwan/modusign-client/node_modules/webpack/lib/Compilation.js:448:13)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

@asfktz
Copy link
Owner

asfktz commented Jul 11, 2017

Hi @taehwanno,
Thanks for the insight!

Interesting, even if I set webpack as a peerDependency of autodll-webpack-plugin like so:

"peerDependencies": {
    "webpack": ">= 2.0.0"
}

I still get this error on angular-starterkit.

But like in your case, upgrading to webpack v3 solves it.

I'll dive into it to see what I can do to fix it from my side.

@cgatian also wanted you to know that your original issue with the multiple instances was fixed in 0.1.0

@asfktz
Copy link
Owner

asfktz commented Jul 11, 2017

@taehwanno are you using awesome-typescript-loader?

I noticed that it works with webpack 2.6.1 if I replace awesome-typescript-loader with ts-loader

I released a new version. webpack is now a peerDependency.

@taehwanno
Copy link

are you using awesome-typescript-loader?

Nope. When I retry autodll-webpack-plugin with v0.2.1 in my private project, script succeeds without errors.

asfktz referenced this issue Jul 12, 2017
@asfktz
Copy link
Owner

asfktz commented Jul 12, 2017

@taehwanno oh, that's good news (:

@asfktz
Copy link
Owner

asfktz commented Jul 12, 2017

@cgatian is updating to webpack v3 an option for you?

@cgatian
Copy link
Author

cgatian commented Jul 12, 2017 via email

@asfktz
Copy link
Owner

asfktz commented Jul 12, 2017

@cgatian I already did it in my fork, so I created a PR for you to use if you like:
PatrickJS/PatrickJS-starter#1828

@asfktz asfktz changed the title Two AutoDLLPlugin's overwrite each other Fixed: Two AutoDLLPlugin's overwrite each other Jul 14, 2017
@asfktz asfktz removed the bug label Jul 14, 2017
@swernerx
Copy link

swernerx commented Jul 18, 2017

Just want to add that double instances of the plugin could also be relevant when using the multi compiler feature of Webpack. We use that for efficient SSR (universal) bundling. See also #43

I would be fine to pass over some specific naming for the caching/output. I does the same already for webpacks new Cache-Loader:

  {
    loader: "cache-loader",
    options: {
      cacheDirectory: resolve(ROOT, `.cache/loader-${CACHE_HASH}-${config.target}-${config.env}`)
    }
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants