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

[thescientist13]

as observed in the webpack v4 upgrade issue, it seems that as part of the 1.0.0 release of critical the package migrated to using Headless Chrome from PhantomJS, which means all build environments (local, CI, etc) have to have support for puppeteer or else builds will break silently.

There's not much for the plugin team to do per se as it will be incumbent on consumers to provide the "correct" build time environment, but I think a couple things could be done to prevent future regressions / awareness

1. introducing some basic CI into the project that tests (and fails, ideally) the build when critical itself fails
2. Add some documentation in the README about the puppeteer dependency

[thescientist13]

so with a basic test webpack configuration

const ExtractTextWebpackPlugin = require('extract-text-webpack-plugin');
const HtmlWebpackCriticalPlugin = require('./index');
const HtmlWebpackPlugin = require('html-webpack-plugin');
const webpack = require('webpack');
const path = require('path');

const OUTPUT_DIR = path.resolve(__dirname, 'build');

module.exports = {
  
  entry: {
    index: './test/cases/generate-critical-css/index.js'
  },

  output: {
    path: OUTPUT_DIR,
    filename: 'html-crtitical-webpack-plugin.js'
  },

  module: {
    rules: [{
      test: /\.css$/,
      use: ExtractTextWebpackPlugin.extract({
        use: ['css-loader']
      })
    }]
  },

  plugins: [
    new HtmlWebpackPlugin(),

    new ExtractTextWebpackPlugin('styles.[chunkhash].css'),
    
    new HtmlWebpackCriticalPlugin({
      base: OUTPUT_DIR,
      src: 'index.html',
      dest: 'index.html',
      inline: true
    })
  ]
};

and using bare bones node 8.9.4 Docker configuration in our repo (using CircleCI)

version: 2
jobs:
  build:
    docker:
      - image: circleci/node:8.9.4

we can see the build doesn't complete

for comparison locally I get this output when running the same command (npm run ci)

$ npm run ci

> [email protected] ci /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin
> webpack --config ./webpack.config.ci.js

Hash: 6250bf09a859aafabd31
Version: webpack 3.11.0
Time: 1522ms
                           Asset       Size  Chunks             Chunk Names
html-crtitical-webpack-plugin.js    2.92 kB       0  [emitted]  index
 styles.e13c0bda710db7138687.css  329 bytes       0  [emitted]  index
                      index.html  267 bytes          [emitted]
   [0] ./test/cases/generate-critical-css/index.js 21 bytes {0} [built]
   [1] ./test/cases/generate-critical-css/index.css 41 bytes {0} [built]
    + 1 hidden module
Child html-webpack-plugin for "index.html":
     1 asset
       [2] (webpack)/buildin/global.js 509 bytes {0} [built]
       [3] (webpack)/buildin/module.js 517 bytes {0} [built]
        + 2 hidden modules
Child extract-text-webpack-plugin node_modules/extract-text-webpack-plugin/dist node_modules/css-loader/index.js!test/cases/generate-critical-css/index.css:
       [0] ./node_modules/css-loader!./test/cases/generate-critical-css/index.css 508 bytes {0} [built]
        + 1 hidden module

[thescientist13]

As preliminary test, I I upgraded my project here to use an image (one of my own that has NodeJS + needed puppeteer O.S. libs) and it is building as expected now.

I've since updated my PR and it is also passing now.

I think we should try and get CircleCI setup for this repo and then submit my PR to this repo.

Thoughts?

[anthonygore]

Totally. I didn't think this plugin would get so complicated, lol.

[anthonygore]

From what I can see, where we're currently stuck is that your PR to upgrade to Webpack 4 appears to fail tests. If we go down this path, do you believe we'll get over that hurdle?

[thescientist13]

Hey @anthonygore.,

Sorry for so many threads, it took a while to get to the bottom of everything, and for better or worse, I made new issues to try and track / isolate each one, but essentially it all stems from the main issue identified in the description, which is that all consumers of critical have to be able to support the operating system runtime needed to support headless chrome (puppeteer).

This is an operating system level change, nothing we can do at the plugin level per se aside from (IMO)

1. Setup CI (CircleCI is free) for this project so as to establish / document basic expectations of what is needed by consumers to use this plugin at build time (i.e. puppeteer dependency) - critical 1.0.0 upgrade introduced (undocumented?) breaking change / dependency on puppeteer (Headless Chrome) #14 (would close this issue).
2. Number 1 should lead us down to path of reliable unit testing once the environment is normalized. Contributors to this repo would need to setup puppeteer locally on their own machine, fyi.
3. Complete the webpack v4 upgrade (webpack v4 compatibility upgrade #12 )
4. Put out release candidate
5. Put out release
6. ???
7. 💰