webpack v4 compatibility upgrade #12

thescientist13 · 2018-03-12T15:48:39Z

Related Issue

resolves #9 , to upgrade the project to support webpack v4

Summary

Started the process of upgrading to webpack v4. Got a couple expected errors from HtmlWebpackPlugin and ExtractTextWebpackPlugin each after upgrading webpack but was able to resolve those easily by upgrading them as well.

Changes

Updated to webpack@^4.0.0 (4.8.2) and installed webpack-cli, which is needed to run webpack from the command line now
Migrated from compiler.plugins API to compiler.hooks API (only "main" change)
Migrated to MiniCssExtractPlugin (deprecated usage of ExtractTextWebpackPlugin)
Updated to latest version of HtmlWebpackPlugin
Updated specs / asserts accordingly based on newer observed behavior I saw (which I am taking for face value, but please feel free to suggest a different approach).
Set node engine in package.json to `>=6.0.0"
Updated README

TODO

resolve conflicts
Upgrade to MiniCssExtracttPlugin
Set node engine in package.json

Misc

Note: This since been resolved since this PR was through a correction in a CSS comment in index.css. webpack wasn't outputting build files as a result, hence the ENOENT error👇

  HtmlCriticalWebpackPlugin Cases: Generate Critical CSS
    minimum configuration
      Unhandled rejection Error: ENOENT: no such file or directory, open '/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/test/cases/generate-critical-css/build/index.html'
    at Object.fs.openSync (fs.js:646:18)
    at Object.fs.readFileSync (fs.js:551:33)
    at webpack (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/test/cases/generate-critical-css/generate-critical-css.spec.js:17:36)
    at hooks.done.callAsync.err (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/webpack/lib/Compiler.js:152:13)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/Hook.js:35:21)
    at onCompiled (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/webpack/lib/Compiler.js:150:21)
    at hooks.afterCompile.callAsync.err (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/webpack/lib/Compiler.js:470:14)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/Hook.js:35:21)
    at compilation.seal.err (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/webpack/lib/Compiler.js:467:30)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeAssets.callAsync.err (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/webpack/lib/Compilation.js:957:35)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/Hook.js:35:21)
    at hooks.optimizeChunkAssets.callAsync.err (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/webpack/lib/Compilation.js:948:32)
    at _err0 (eval at create (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:11:1)
    at /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/uglifyjs-webpack-plugin/dist/index.js:266:11
    at step (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/uglifyjs-webpack-plugin/dist/uglify/index.js:90:11)
    at done (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/uglifyjs-webpack-plugin/dist/uglify/index.js:99:22)
    at tryCatcher (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._resolveCallback (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:432:57)
    at Promise._settlePromiseFromHandler (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:524:17)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._resolveCallback (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:432:57)
    at Promise._settlePromiseFromHandler (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:524:17)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:582:21)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._resolveCallback (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:454:14)
    at Promise._settlePromiseFromHandler (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:524:17)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/promise.js:638:18)
    at /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/bluebird/js/release/nodeback.js:42:21
    at /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/graceful-fs/graceful-fs.js:121:16
    at /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/node_modules/graceful-fs/graceful-fs.js:43:10
    at FSReqWrap.oncomplete (fs.js:135:15)

Resources

Here are some good links and docs I've found re: upgrading to v4 for plugins / loaders

thescientist13 · 2018-03-12T18:19:38Z

So I have been banging my head against this thinking its been something to do with migrating to the hooks API (could still be an issue there, just got stuck somewhere sooner along the path it seems).

I have determined that (possibly due to webpack-cli being moved out of core webpack?) the upgrade to v4 is "breaking" tests by not actually generating a build into the build directory anymore.

In this branch, I have only updated webpack to v4 and commented all the other loaders and plugins out of the test webpack config, as well as the assertions within the generate-css spec file, and while mocha runs and the tests pass, there is no output to the filesystem.

describe('HtmlCriticalWebpackPlugin Cases: Generate Critical CSS', () => {
  const buildDirectory = path.resolve(__dirname, 'build');

  describe('minimum configuration', () => {

    it('should generate the expected critical inline <style> tag', (done) => {
      webpack(webpackConfig, () => {
        // const indexHtmlString = fs.readFileSync(`${buildDirectory}/index.html`, {encoding: 'utf8'});
        // const indexHtmlDom = new JSDOM(indexHtmlString);
        // const inlineStyleTags = indexHtmlDom.window.document.querySelectorAll('style');
        
        // assert.equal(inlineStyleTags.length, 1);
        // assert.equal(inlineStyleTags[0].getAttribute('type'), 'text/css');

        done();
      });
    });

    //etc
  })
})

$ npm test

> html-critical-webpack-plugin@1.1.0 test /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin
> rimraf ./test/cases/**/build/** && mocha ./test/**/*.spec.js

spec webpackConfig { mode: 'production',
  entry:
   { index: '/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/test/cases/generate-critical-css/index.js',
     main: '/Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin/test/cases/generate-critical-css/main.js' },
  plugins: [] }


  HtmlCriticalWebpackPlugin Cases: Generate Critical CSS
    minimum configuration
      ✓ should generate the expected critical inline <style> tag (428ms)
      - should generate the expected critical <link> tag
      - should generate the expected critical <noscript> tag

  HtmlCriticalWebpackPlugin
    webpack hooks
      ✓ HtmlCriticalWebpackPlugin has the required apply method
      ✓ HtmlCriticalWebpackPlugin has the required emit method


  3 passing (433ms)
  2 pending

By comparison, using the webpack-cli directly shows us output we would expect if we didn't have a loader configured for CSS

$ ./node_modules/.bin/webpack-cli --config ./test/cases/generate-critical-css/webpack.config.js
Hash: 4d2d7501158ffb5ed2af
Version: webpack 4.1.1
Time: 94ms
Built at: 2018-3-12 14:19:20
 2 assets
Entrypoint index = index.js
Entrypoint main = main.js
   [0] ./test/cases/generate-critical-css/main.css 200 bytes {0} [built] [failed] [1 error]
   [1] ./test/cases/generate-critical-css/main.js 101 bytes {0} [built]
   [2] ./test/cases/generate-critical-css/index.css 226 bytes {1} [built] [failed] [1 error]
   [3] ./test/cases/generate-critical-css/index.js 104 bytes {1} [built]

ERROR in ./test/cases/generate-critical-css/main.css
Module parse failed: Unexpected token (1:0)
You may need an appropriate loader to handle this file type.
| .card {
|   background: none !important;
|   border: none !important;
 @ ./test/cases/generate-critical-css/main.js 3:0-20

ERROR in ./test/cases/generate-critical-css/index.css
Module parse failed: Unexpected token (1:0)
You may need an appropriate loader to handle this file type.
| .lazyload-wrapper .card-img {
|   border-radius: 0 !important;
|   min-height: 233px!important;
 @ ./test/cases/generate-critical-css/index.js 3:0-21

thescientist13 · 2018-03-12T22:34:12Z

I figured out the issue why webpack wasn't outputting files ☝️ it was because i had a JS style comment in index.css 😕

.fade-appear.fade-appear-active {
  opacity: 1;
  transition: opacity 600ms ease-in; //ms should match the value of transitionAppearTimeout
}

changed it to /* .... */ and it's working (i.e. compiles a build and outputs files).

This PR is now ready for review.

Additional considerations as other PR(s) most likely:

Convert plugin to ES6 Class syntax?
Set a mode option in the root webpack.config.js when building, i.e. yarn run build?
Should some linting be added, i.e. ESLint?
Create a release task that can lint, run tests, and build?

anthonygore · 2018-03-25T06:51:08Z

Great work on this, @thescientist13

A few comments:

The test should generate the expected critical inline <style> tag is timing out for me.
I noticed in test/cases/generate-critical-css/main.css there are some invalid colors e.g. $gold. I think this was from your previous PR.
A small thing, but, when we identify the plugin it looks like the convention it to use kebab case so in index.js 'HtmlCriticalWebpackPlugin' should probably be 'html-critical-webpack-plugin'

Regarding your other suggestions, I'm a yes to all of those. A caveat about point 1, though, we'll need to find out the minimum supported version of Node for Webpack v4 and ensure classes are supported. Or transpile.

anthonygore · 2018-03-25T07:02:52Z

What will we do about backwards compatibility? My thinking is that we tell people to use the previous version. If you agree, perhaps include an update to README.

Also, I think we should we bump the version to 2.0.0 in this PR.

thescientist13 · 2018-03-26T15:16:20Z

thanks for the feedback @anthonygore , I'll put some into this today.

thescientist13 · 2018-03-26T16:17:08Z

Made some updates, let me know your thoughts @anthonygore

The test should generate the expected critical inline <style> tag is timing out for me.

I think this is probably just .a race condition and hitting the default mocha timeout, so I bumped the mocha timeout to 5000ms in package.json, let me know if that helps on your end.

I noticed in test/cases/generate-critical-css/main.css there are some invalid colors e.g. $gold. I think this was from your previous PR.

Good catch! I changed those instances to just normal hex colors

A small thing, but, when we identify the plugin it looks like the convention it to use kebab case so in index.js 'HtmlCriticalWebpackPlugin' should probably be 'html-critical-webpack-plugin'

Not that I have a strong preference one way or the other, but the docs and guides I followed seemed to
favor CamelCase

Regarding your other suggestions, I'm a yes to all of those. A caveat about point 1, though, we'll need to find out the minimum supported version of Node for Webpack v4 and ensure classes are supported. Or transpile.

webpack v4 dropped support for Node <= v4.
https://github.com/webpack/webpack/releases/tag/v4.0.0

and now supports >= 6, which has support for Class, so need to transpile 👌
https://node.green/#ES2015-functions-class

Made an issue for tracking all the misc. refactoring items unrelated to this PR - #13

thescientist13 · 2018-03-26T16:35:32Z

Also, not sure if this is the PR we want to introduce mini-css-extract-webpack-plugin with, but I am happy to include it here.

anthonygore · 2018-03-27T06:01:31Z

Unfortunately the failing test is not a race condition. When I inspected the outputted HTML, there's no style tag! I'm not sure why the tests don't correctly fail, either, they just timeout.

I'm cool with CamelCase if that's a convention.

All the rest is fine, too.

anthonygore · 2018-03-27T06:05:22Z

Actually, I just tried to run the tests on the master branch and it's the same problem.

I haven't investigated deeply, but I did try to log any errors from the API i.e.:

webpack(webpackConfig, (err, stats) => {
  console.log(err);
});

But there are no errors. Strange.

thescientist13 · 2018-03-27T15:25:48Z

@anthonygore
What version of Node are you on? I am on v8.9.4, but I would at least see if you are on something >= 6.x (minimum version supported by webpack v4) and see if that helps. 🤔

anthonygore · 2018-03-27T20:14:56Z

I tried node versions 4, 6, 8 and 9....same problem with all

thescientist13 · 2018-03-28T14:03:11Z

@anthonygore
I started a CircleCI build on my own fork PR and seems to corroborate the issue (please confirm if it is similar to what you are seeing (I branched off master)

will continue to investigate. For the record, I am using OSX.

anthonygore · 2018-03-29T07:08:03Z

Yes, OSX for me too

anthonygore · 2018-05-12T09:20:24Z

Hey @thescientist13, after your great work getting the docker environment created, I think we're almost ready to merge this. What I think needs to be done is:

Resolve conflicts
Bump version of plugin to 2.0
Add engines to package.json and ensure Node >6 is used

[edit]

Actually...

I don't think we should merge this unless it includes mini-css-extract-plugin. Since EWTP is not going to be supported by Webpack 4, we'll have instant problems. What do you think?

thescientist13 · 2018-05-12T11:54:43Z

@anthonygore
Agreed on deprecating ETWP. I'll get this PR all fixed up today. 👍

…xt-webpack-plugin

thescientist13 · 2018-05-12T12:31:43Z

All set for review @anthonygore !

Question, should this be updated?

Had a question about the build task also (let me know if a new issue makes more sense). Could you explain a little more what it's used for? From my perspective, I think it might just be sufficient to do this in package.json?

"main": "index.js",

I'm not sure how much transpiling of node specific packages is needed in most use cases, but I think it's generally become standard practice to let consumers transpile if they need to, and let authors just publish the source. Let me know your thoughts!

initial WIP of webpack v4 compatibility upgrade

670d31c

thescientist13 added 5 commits March 12, 2018 18:19

fixed no build output bug

0ec06cf

use dist directory convention

b79c2f1

passing tests

900cb51

test passing

43d7adb

deprecate use of plugins API

aa5bdb5

thescientist13 added 3 commits March 12, 2018 18:57

use build directory

9c69916

enable all tests

6d1b2a3

fix build task

559d61a

thescientist13 changed the title ~~webpack v4 compatibility upgrade [WIP]~~ webpack v4 compatibility upgrade Mar 12, 2018

remove extra commits

77dc853

thescientist13 mentioned this pull request Mar 13, 2018

upgrade to webpack v4 ProvidenceGeeks/website-frontend#142

Merged

7 tasks

thescientist13 added 2 commits March 26, 2018 12:04

increase mocha timeout

708d042

fix css syntax

091951c

thescientist13 mentioned this pull request Mar 26, 2018

recommendations for refactoring / technical improvements (webpack v4) #13

Closed

made timeout 5 seconds

4badc4e

thescientist13 mentioned this pull request Mar 28, 2018

circle config thescientist13/html-critical-webpack-plugin#1

Closed

2 tasks

thescientist13 mentioned this pull request Apr 17, 2018

critical 1.0.0 upgrade introduced (undocumented?) breaking change / dependency on puppeteer (Headless Chrome) #14

Closed

thescientist13 added 5 commits May 12, 2018 08:05

merging in latest from master

2de14b5

upgrade webpack and use mini-css-extract-plugin instead of extract-te…

49e46d2

…xt-webpack-plugin

document tasks

4b511d2

add engines node to package.json

10e1205

rename section

5cf8d57

anthonygore merged commit b89192b into anthonygore:master May 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

webpack v4 compatibility upgrade #12

webpack v4 compatibility upgrade #12

thescientist13 commented Mar 12, 2018 •

edited

Loading

thescientist13 commented Mar 12, 2018 •

edited

Loading

thescientist13 commented Mar 12, 2018 •

edited

Loading

anthonygore commented Mar 25, 2018

anthonygore commented Mar 25, 2018

thescientist13 commented Mar 26, 2018

thescientist13 commented Mar 26, 2018 •

edited

Loading

thescientist13 commented Mar 26, 2018

anthonygore commented Mar 27, 2018

anthonygore commented Mar 27, 2018

thescientist13 commented Mar 27, 2018 •

edited

Loading

anthonygore commented Mar 27, 2018

thescientist13 commented Mar 28, 2018 •

edited

Loading

anthonygore commented Mar 29, 2018

anthonygore commented May 12, 2018 •

edited

Loading

thescientist13 commented May 12, 2018

thescientist13 commented May 12, 2018 •

edited

Loading

webpack v4 compatibility upgrade #12

webpack v4 compatibility upgrade #12

Conversation

thescientist13 commented Mar 12, 2018 • edited Loading

Related Issue

Summary

Changes

TODO

Misc

Resources

thescientist13 commented Mar 12, 2018 • edited Loading

thescientist13 commented Mar 12, 2018 • edited Loading

anthonygore commented Mar 25, 2018

anthonygore commented Mar 25, 2018

thescientist13 commented Mar 26, 2018

thescientist13 commented Mar 26, 2018 • edited Loading

thescientist13 commented Mar 26, 2018

anthonygore commented Mar 27, 2018

anthonygore commented Mar 27, 2018

thescientist13 commented Mar 27, 2018 • edited Loading

anthonygore commented Mar 27, 2018

thescientist13 commented Mar 28, 2018 • edited Loading

anthonygore commented Mar 29, 2018

anthonygore commented May 12, 2018 • edited Loading

thescientist13 commented May 12, 2018

thescientist13 commented May 12, 2018 • edited Loading

thescientist13 commented Mar 12, 2018 •

edited

Loading

thescientist13 commented Mar 12, 2018 •

edited

Loading

thescientist13 commented Mar 12, 2018 •

edited

Loading

thescientist13 commented Mar 26, 2018 •

edited

Loading

thescientist13 commented Mar 27, 2018 •

edited

Loading

thescientist13 commented Mar 28, 2018 •

edited

Loading

anthonygore commented May 12, 2018 •

edited

Loading

thescientist13 commented May 12, 2018 •

edited

Loading