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

Webpack v4.0.0-alpha support #823

Closed
wants to merge 28 commits into from
Closed

Webpack v4.0.0-alpha support #823

wants to merge 28 commits into from

Conversation

Graham42
Copy link
Contributor

@Graham42 Graham42 commented Dec 12, 2017

This is a rough go at the code to make tests pass for supporting webpack@4.0.0-alpha-4.

I'm not sure that this can be merged with out major version bump since webpack now requires node version >=6 and html-webpack-plugin currently still supports 4 and 5.
Edit: This is actually probably fine, if people are using webpack the intersection of supported node versions is >=6, I don't see a need to change the required node version for this plugin.

Status as of webpack@4.0.0-alpha-4:

  • Caching Tests passing in development mode
  • Example fixture Tests passing in production mode
  • Basics Tests passing in development mode

lib/compiler.js Outdated
hash: childCompilation.hash,
chunk: entries[0]
})
: compilation.mainTemplate.hooks.assetPath.call(outputOptions.filename, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use compilation.mainTemplate.getAssetPath instead

index.js Outdated
@@ -71,7 +85,12 @@ HtmlWebpackPlugin.prototype.apply = function (compiler) {
// Sort chunks
chunks = self.sortChunks(chunks, self.options.chunksSortMode);
// Let plugins alter the chunks and the chunk sorting
chunks = compilation.applyPluginsWaterfall('html-webpack-plugin-alter-chunks', chunks, { plugin: self });
if (compilation.applyPluginsWaterfall) {
Copy link
Contributor

Choose a reason for hiding this comment

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

reverse the order here: Try new API first, fallback to old API

index.js Outdated
@@ -313,7 +332,12 @@ HtmlWebpackPlugin.prototype.addFileToAssets = function (filename, compilation) {
})
.then(function (results) {
var basename = path.basename(filename);
compilation.fileDependencies.push(filename);
if (compilation.fileDependencies.push) {
Copy link
Contributor

Choose a reason for hiding this comment

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

reverse the order here: Try new API first, fallback to old API

index.js Outdated
return _.extend(pluginArgs, result);
});
};
if (compilation.applyPluginsAsyncWaterfall) {
Copy link
Contributor

Choose a reason for hiding this comment

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

reverse the order here: Try new API first, fallback to old API

@Graham42
Copy link
Contributor Author

Graham42 commented Dec 12, 2017

going to see if there's a better way to view the diffs for fixture tests. the current output is not easy to tell what's wrong. 😕

All of the tests now pass on my machine, but there's a couple still failing in travis.

But don't test webpack 4 on old node versions since it now requires 6 or
later.
webpack 4 drops the 'loaders' key on the config
webpack has a completely new plugin interface. See changes to
wepack/tapable for in-depth.
In the future the file-loader should move away from loadContext.options
and this would not be needed.
Webpack 4 now requires either 'production', 'development', or 'none'.
The new method with AsyncSeriesWaterfallHook already merges the initial
value with the result so there's no need to worry about the result not
being defined.
Recompiling and caching only really makes sense for development builds.
toposort is non-deterministic when there's a tie. Needed to allow for
either result in the test.

This happens when A and B both depend on C, dependency graph looks like:

      A
     /
    C
     \
      B

Ideally we could pass a tie breaker sort function to toposort to fix
this.
Previously was just doing a strait string compare which was very hard to
tell what the differences causing the failing were.
@Graham42
Copy link
Contributor Author

Rebased to clean up the commits. Included the changes suggested from @sokra, thanks for the review.

Tests all pass now 🎉 , However I've made some assumptions which I'd like feedback on.

I think the caching tests only make sense for mode = 'development' because that's how people will be running dev servers that require recompiling.

I think mode = 'production' makes sense for the fixture tests from looking a the other fixture files. I tried generating both production and development variations, and the production ones look more like what's there for webpack 3.

The basic tests I think should be run for both modes but I'm not sure.

@donaldpipowitch
Copy link

donaldpipowitch commented Dec 22, 2017

Thank you for the PR. I just tried it against the webpack v4 alpha in a small example and it worked without problems. ✌️

Just got one deprecation warning:

(node:52664) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
    at HtmlWebpackPlugin.apply (./node_modules/html-webpack-plugin/index.js:59:12)

minification is now on by default for production mode.
@Graham42 Graham42 changed the title [WIP] Webpack v4 support Webpack v4.0.0-alpha support Jan 8, 2018
@Graham42
Copy link
Contributor Author

Graham42 commented Jan 8, 2018

This good to go. The deprecation warnings can be fixed in a separate PR. As it is this PR as it is enables compatibility with webpack-4.0.0-alpha.4. cc @jantimon @TheLarkInn

@Graham42
Copy link
Contributor Author

Graham42 commented Jan 8, 2018

Travis is timing out on network requests, going to push again to trigger build

@alexandernst
Copy link

So... merge?

@TheLarkInn
Copy link
Collaborator

@Graham42 this looks great to me as well.

@jantimon would you like to release this under '@next' tag similar to how we are shipping alphas? Let me know where we can help.

@niieani
Copy link

niieani commented Jan 17, 2018

Are there any ETAs on this getting released as an alpha/next/beta?

@Graham42
Copy link
Contributor Author

Note that this plugin breaks with webpack@4.0.0-alpha-5 because of reliance on old common chunk behaviour.

Not sure when I'll have a chance to look at that, but if someone else wants to work on it, could probably build on top of this branch.

@TheLarkInn
Copy link
Collaborator

Update: currently jantimon is in Asia for work right now until first week of March. We've forked html-webpack-plugin for now to have the PR sent against there just to ensure people can actually have a chance to test and use this package prior to v4

Would someone like to take the PR and revase against webpack-contrib/html-webpack-plugin ?

@quantizor
Copy link

Some of the tests also are checking for commonschunk, which no longer exists.

@Graham42
Copy link
Contributor Author

Continuing this PR over at https://github.com/webpack-contrib/html-webpack-plugin/pull/3

webpack-4.0.0-alpha.5 introduced a large refactor to chunks. This is a
corresponding update to handle the new structure.

This change introduces an API change to `chunksorter.dependency`. It now
requires an additional parameter `chunkGroups` that is available from
the compilation object.

To read more about the CommonsChunkPlugin changes see:
- webpack/webpack#6357
- https://gist.github.com/sokra/1522d586b8e5c0f5072d7565c2bee693
Tests were green before this so can have higher confidence in this
change.
Wanted to update the test fixtures to not use minification, but needed
to go back to a stable state to do that.
Appear to all be internal webpack changes.

It is a bit painful to review. Certain parts may be easier to see the
changes by using the --word-diff-regex flag. Example:

    git diff --word-diff-regex=[^[:space:]\"\)]+
Need a webpack 4 compatible version.
Perf improvement.

Using getParents() causes a full iteration over the parents Set to get
an array. Then using map() is a second iteration over the resulting
array. By using Array.from(iterable, mapFn) we only iterate once over
the internal data structure of parents.
@hanxue
Copy link

hanxue commented Feb 22, 2018

After installing the changes from

npm i -D git://github.com/Graham42/html-webpack-plugin.git#feat/webpack-4

I still get the following error

TypeError: SyncWaterfallHook is not a constructor
    at /a/b/c/node_modules/html-webpack-plugin/index.js:50:56
    at SyncHook.eval [as call] (eval at create (/a/b/c/node_modules/webpack/node_modules/tapable/lib/HookCodeFactory.js:17:12), <anonymous>:11:1)

Manage to resolve this error by installing the latest tapable v1.0.0

@niemyjski
Copy link

Any updates on this?

@caseyWebb
Copy link

@niemyjski

🛑 If you use HtmlWebpackPlugin

For this release, we gave the ecosystem a month to upgrade any plugins or loaders to use the new webpack 4 API’s. However, Jan Nicklas has been away with work obligations and therefore, we have provided a patched fork of html-webpack-plugin . For now you can install it by doing the following:

$ yarn add html-webpack-plugin@webpack-contrib/html-webpack-plugin

When Jan returns from overseas work at the end of the month, we plan to merge our fork upstream into jantimon/html-webpack-plugin ! Until then, if you have any issues, you can submit them here!

via: https://medium.com/webpack/webpack-4-released-today-6cdb994702d4

@deadcoder0904
Copy link

deadcoder0904 commented Feb 28, 2018

@niemyjski This PR works I tried & you can install it like that on the Medium article or like this

@jantimon jantimon closed this Mar 1, 2018
@jantimon
Copy link
Owner

jantimon commented Mar 1, 2018

Merged - ty so much @Graham42

@ankibalyan
Copy link

realeasing something? html-webpack-plugin@4.0.0 or html-webpack-plugin@next?

@deadcoder0904
Copy link

Just do npm i --save-dev html-webpack-plugin or yarn add --dev html-webpack-plugin

Its been released on npm

@lock
Copy link

lock bot commented May 31, 2018

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.