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

[WIP]: webpack 4 support #106

Merged
merged 18 commits into from
Jun 2, 2018
Merged

[WIP]: webpack 4 support #106

merged 18 commits into from
Jun 2, 2018

Conversation

mhheise
Copy link
Contributor

@mhheise mhheise commented Feb 23, 2018

WIP: https://github.com/mhheise/autodll-webpack-plugin/tree/feat/webpack-4

Do not merge yet, please!

I would appreciate any feedback you may have 😄

Todo:

  • Upgrade to webpack 4 and other related dependencies across package.json files.
  • webpack
  • webpack-cli (now required to run webpack from the command line)
  • webpack-dev-server
  • extract-text-webpack-plugin (now a dependency of html-webpack-plugin)
  • html-webpack-plugin
  • Use compiler.hooks.<identifier>.tap('name', step) instead of compiler.plugin(<identifier>, step).
  • Use compiler.hooks feature detection to support webpack 3 and webpack 4.
  • Use Set() for compilationDependencies (still need to make this backwards compatible).
  • Implement new mode configuration property in webpack configs.
  • Update tests to use the new webpack 4 strategy (and perhaps test backwards compatibility to webpack 3).
  • Extra credit: Update examples to babel 7 and react 16.

Blockers:

  • webpack-contrib/html-webpack-plugin#3 stable release

@asfktz
Copy link
Owner

asfktz commented Feb 27, 2018

Hi again, @mhheise!
First, it's really cool what's you're doing here, and much appreciated 😊

About the file you mentioned:

'C:\Git\autodll-webpack-plugin\examples\recommended\node_modules\.cache\autodll-webpack-plugin\development_instance_0_1dc0c813842ea3b964c005e53b6b3ea3\vendor.manifest.json'

This file, generated by the plugin, or the be more accureate, by the DllPlugin, here:
https://github.com/mhheise/autodll-webpack-plugin/blob/68594cb2e689d7116bd7e7c669a40e69f5a91a36/src/createConfig.js#L45

In the end, this plugin is just an abstraction for the DllPlugin & DllReferencePlugin plugin.

I've noticed that it complains about:
TypeError: Cannot read property 'compilation' of undefined

This happens when it calls new DllReferencePlugin(...).apply(compiler)
where the manifestPath file, is the file you mentioned.

Object.keys(dllConfig.entry)
      .map(getManifestPath(settings.hash))
      .forEach(manifestPath => {
        new DllReferencePlugin({
          context: context,
          manifest: manifestPath, 
        }).apply(compiler);
      });

That's very interesting... you see, for the DLL to work you have to add the DllReferencePlugin plugin to the user's config (That is how the original DllPlugin work)

At that point in the AutoDLL plugin's life cycle, the manifest file does not generated yet.
But, at least for webpack 3, the only way the add additional plugins to the original config was to add them at that stage before any lifecycle hook is triggered.

The original DllReferencePlugin had no trouble referencing a manifest file does not exist yet.
But it seems like it does now /:

Btw, here a little tip, if you want to debug the plugin with devtools, you can:

  1. Open a terminal tab
  2. Go to the plugins directory and run: npm run build:watch
  3. Open another terminal tab
  4. Go to the ./examples/recommended and run node --inspect --inspect-brk node_modules/.bin/webpack
  5. Open chrome, and go to chrome://inspect
  6. Click Open dedicated DevTools for Node

And you should be able to debug the plugin now (:

Also, you can install NIM chrome extensions which will popup devtools for you automaticity

src/plugin.js Outdated
);
const beforeCompile = params => {
const dependencies = new Set(params.compilationDependencies);
[...dependencies].filter(path => !path.startsWith(cacheDir));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you missed a return there (:

@mhheise
Copy link
Contributor Author

mhheise commented Mar 5, 2018

@asfktz: Thank you so much for the detailed review! I think I am stuck at this point -- I am not sure where and how I would ensure that the manifest file is generated.

EDIT: Perhaps we need an enhancement in webpack to add a hook to the DLL and DLL reference plugins, like what is present in HTML plugin, to indicate when the manifest has been generated?

@asfktz
Copy link
Owner

asfktz commented Mar 6, 2018

EDIT: Perhaps we need an enhancement in webpack to add a hook to the DLL and DLL reference plugins, like what is present in HTML plugin, to indicate when the manifest has been generated?

Hmm.. not sure I understand, can you show me an example? thanks!

@mhheise
Copy link
Contributor Author

mhheise commented Mar 6, 2018

@asfktz
Copy link
Owner

asfktz commented Mar 14, 2018

Hi @mhheise! Sorry for the delay, I got busy lately.
Lets make it happen 😄

I've played around with your code to see what I can help with.
The good news it that it works! I really didn't do much. its your magic.
The only thing I did was to comment out html-webpack-plugin on the user side.
Seems like it's not working properly.

Here the changes I made: b2db631...mhheise-feat/webpack-4

The interesting parts are examples/recommended/webpack.config.js and src/plugin.js

running npm run build and npm start in the recommended works.
The DLL was built, but no index.html of course.

I haven't got the time to dig into webpack 4 yet,
but I read something about html-webpack-plugin is not compatible with it?

@mhheise
Copy link
Contributor Author

mhheise commented Mar 15, 2018

@asfktz Thank you!

html-webpack-plugin is now compatible with webpack@4 (the latest version at this time is 3.0.6, with webpack@4 supported in 3.0.0 and greater). The reason I originally used webpack-contrib/html-webpack-plugin was that the webpack-contrib organization forked html-webpack-plugin until the maintainer returned and the support could be merged back into the upstream repository.

I think I have most of the webpack@4 support complete, but the tests are timing out. I'm pretty sure I'm missing something but I haven't been able to figure out what.

I apologize for all of the back-and-forth on this PR -- I'm still new to JS and to open-source contributions! I do appreciate all of the help you've provided so far.

@emmenko
Copy link

emmenko commented Mar 16, 2018

Cool! Looking forward to seeing this working. If it's ready to be tested I can try it out on my project before we merge this.

@asfktz
Copy link
Owner

asfktz commented Mar 17, 2018

I apologize for all of the back-and-forth on this PR -- I'm still new to JS and to open-source contributions! I do appreciate all of the help you've provided so far.

No worries, @mhheise! we're all here to learn 😄

I've found a bug which may relate to the tests being timed out.

The plugin exposes a hook called autodll-stats-retrieved
It used mostly for testing purposes.

I've noticed that while running the recommended example, it throws:

recommended git:(pr/106) ✗ npm run build

> basic@1.0.0 build /Users/asafkatz/dev/autodll-webpack-plugin/examples/recommended
> webpack

(node:36787) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
AutoDllPlugin: is valid cache? false
AutoDllPlugin: cleanup
AutoDllPlugin: compile
TypeError: compiler.applyPlugins is not a function
    at /Users/asafkatz/dev/autodll-webpack-plugin/lib/plugin.js:143:20
    at <anonymous>

Which I believe related to those lines:

if (compiler.hooks) {
  compiler.hooks.autodllStatsRetrieved = new SyncHook(['stats', 'source']);
  compiler.hooks.autodllStatsRetrieved.call(stats, source);
} else {
  compiler.applyPlugins('autodll-stats-retrieved', stats, source);
}

Steps to reproduce:

  1. Navigate to the recommended example in the console cd ./examples/recommended
  2. Make sure the cache is empty by running: rm -rf node_modules/.cache
  3. run npm run build

You should see the above error.

@asfktz
Copy link
Owner

asfktz commented Mar 17, 2018

Hi, @emmenko. Thanks!
Your help is very welcome 🙏

We'll let you know when its ready.

@akuznetsov-gridgain
Copy link

Will autodll bring some speedup for webpack4?
Assuming wp4 has built in chunks?

@emmenko
Copy link

emmenko commented Apr 3, 2018

Will autodll bring some speedup for webpack4

It would be good to find out yes 👌

@faceyspacey
Copy link
Contributor

TypeError: compiler.applyPlugins is not a function doesn't occur for me. watchRun successfully checks if (compiler.hooks) and uses the new api instead of compiler.applyPlugins.

The whole thing seems to be working great, except html-webpack-plugin, which I personally don't need. If that's the only remaining blocker, I'll look into that and see what's needed there.

@faceyspacey
Copy link
Contributor

faceyspacey commented Apr 5, 2018

ok, i got the html-webpack-plugin to work via 2 small changes. Here's a PR to @mhheise 's original PR.

https://github.com/mhheise/autodll-webpack-plugin/pull/1

...for me the integration tests hang on testing usage with webpack-dev-server. I get this far:

> basic@1.0.0 test /Users/jamesgillmore/codementor/autodll/specs/fixtures/basic
> ava --serial specs/*.integration.js

Detect package.json change
Ensure stats retrieved from the currect source
clean run (cache deleted)
Starting the development server...

AutoDllPlugin: is valid cache? false
AutoDllPlugin: cleanup
AutoDllPlugin: compile

doesn't go any farther than compile

The fixtures have been installed etc. Perhaps the integration helpers have to be updated.

@faceyspacey
Copy link
Contributor

Here it is temporarily if anyone out there wants to give it a try without jumping through any hoops:

https://www.npmjs.com/package/autodll-webpack-plugin-webpack-4
yarn add --dev autodll-webpack-plugin-webpack-4

@faceyspacey faceyspacey mentioned this pull request Apr 5, 2018
@zhbhun
Copy link

zhbhun commented Apr 9, 2018

@faceyspacey HTML plugin hooks should tap after compiler's afterPlugins

refer to other html plugin https://github.com/waysact/webpack-subresource-integrity/blob/master/index.js#L338

@Venryx
Copy link

Venryx commented Apr 23, 2018

Just wanted to mention this plugin looks nice, but I'm using Webpack 4 so hesitant to try until the official version supports it. (thanks to @faceyspacey though for making most of the code changes needed!)

@waterfoul
Copy link

@mhheise What are the chances you merge @faceyspacey's work?

fix($htmlWebpackPlugin): add run hook and use tapAsync for doCompilation
@mhheise
Copy link
Contributor Author

mhheise commented Apr 28, 2018

Thanks @faceyspacey. PR has been merged.

@pleunv
Copy link

pleunv commented May 2, 2018

Here it is temporarily if anyone out there wants to give it a try without jumping through any hoops:
https://www.npmjs.com/package/autodll-webpack-plugin-webpack-4
yarn add --dev autodll-webpack-plugin-webpack-4

Gave this one a try but I'm running into a couple fs-related errors:

× 「wdm」: Error: ENOENT: no such file or directory, open 'c:\<project-dir>\node_modules\.cache\autodll-webpack-plugin\development_instance_0_5a54a620c19768a5faf5a15e78daf557\vendor.manifest.json'
AutoDllPlugin: is valid cache? false
AutoDllPlugin: cleanup
AutoDllPlugin: compile
{ Error: ENOENT: no such file or directory, open 'c:\<project-dir>\node_modules\.cache\autodll-webpack-plugin\development_instance_0_5a54a620c19768a5faf5a15e78daf557\stats.json'
    at c:\<project-dir>\node_modules\autodll-webpack-plugin-webpack-4\lib\createHandleStats.js:19:15
    at c:\<project-dir>\node_modules\autodll-webpack-plugin-webpack-4\lib\createHandleStats.js:56:16
    at <anonymous>
  cause:
   { Error: ENOENT: no such file or directory, open 'c:\<project-dir>\node_modules\.cache\autodll-webpack-plugin\development_instance_0_5a54a620c19768a5faf5a15e78daf557\stats.json'
     errno: -4058,
     code: 'ENOENT',
     syscall: 'open',
     path: 'c:\\<project-dir>\\node_modules\\.cache\\autodll-webpack-plugin\\development_instance_0_5a54a620c19768a5faf5a15e78daf557\\stats.json' },
  isOperational: true,
  errno: -4058,
  code: 'ENOENT',
  syscall: 'open',
  path: 'c:\\<project-dir>\\node_modules\\.cache\\autodll-webpack-plugin\\development_instance_0_5a54a620c19768a5faf5a15e78daf557\\stats.json' }

@zhbhun
Copy link

zhbhun commented May 5, 2018

Base on autodll-webpack-plugin-webpack-4, I got it work with html-webpack-plugin and fix some internal erros.

https://www.npmjs.com/package/autodll-webpack4-plugin

https://github.com/zhbhun/autodll-webpack-plugin

@klassicd
Copy link

klassicd commented May 5, 2018

@zhbhun I'm getting ReferenceError: vendor_b9e331f1a5e536a9a124 is not defined.

@zhbhun
Copy link

zhbhun commented May 6, 2018

@klassicd I cannot reproduce the problem, you could try basic example https://github.com/zhbhun/autodll-webpack-plugin/tree/master/examples/basic.

@Vincz
Copy link

Vincz commented May 23, 2018

Hey guys! Any update on this one?

@Vincz
Copy link

Vincz commented May 23, 2018

Hey @zhbhun, I tried your repo with all the example. It's working fine (except 'performance' and 'recommanded' because of a repository not found error (https://github.com/webpack-contrib/html-webpack-plugin doesn't exist).

I tried in my own project, and like @pleunv , with your repo or @faceyspacey one, I get errors like that:

{ Error: ENOENT: no such file or directory, open '/Volumes/xxx/node_modules/.cache/autodll-webpack-plugin/development_instance_0_82e345bb31c079df964161da1db1abc7/stats.json'

Hope it'll help!

@Vincz
Copy link

Vincz commented May 23, 2018

Ok, I found the problem. The problem occurs when dll compile failed. One of the package was missing so the build failed, and the cache was not generated.

@zhbhun
Copy link

zhbhun commented May 24, 2018

Oh, dll build does't check compilation error.

https://github.com/asfktz/autodll-webpack-plugin/blob/master/src/createCompileIfNeeded.js#L23

if (err) return reject(err);
if (stats.compilation.errors.length) return reject(stats.compilation.errors);

@vovakulikov
Copy link

Hey guys! Any update on this one? 😿

@asfktz
Copy link
Owner

asfktz commented May 30, 2018

I apologize for the long wait.

@mhheise & @faceyspacey, thank you so much for your hard work. It is much appreciated 🙏

I'll take it from here.
In the next few days I'll merge this branch and release a new version to support webpack 4.

@asfktz asfktz merged commit 565040d into asfktz:master Jun 2, 2018
@asfktz
Copy link
Owner

asfktz commented Jun 2, 2018

Done (:

@sudkumar
Copy link

sudkumar commented Jun 4, 2018

Waited for a long time. Thank you. 👍

@mhheise
Copy link
Contributor Author

mhheise commented Jun 6, 2018

@asfktz @faceyspacey Thank you for your help!

And thank you to all who tested this out and waited patiently 👍

@mhheise mhheise deleted the feat/webpack-4 branch June 6, 2018 20:14
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

Successfully merging this pull request may close these issues.