anticipated changes needed for webpack v4 support? #9

thescientist13 · 2018-02-25T19:36:54Z

I'm in the process of upgrading to [email protected] and while working through issues with ExtractTexxtWebpackPlugn which I think is why I am seeing this error in my PR here

(node:15301) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
(node:15301) DeprecationWarning: Tapable.apply is deprecated. Call apply on the plugin directly instead
Error: css should not be empty
    at /Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/penthouse/lib/index.js:308:33
    at Generator.next (<anonymous>)
    at step (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/penthouse/lib/index.js:45:191)
    at /Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/penthouse/lib/index.js:45:437
    at new Promise (<anonymous>)
    at /Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/penthouse/lib/index.js:45:99
    at Promise (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/penthouse/lib/index.js:325:20)
    at new Promise (<anonymous>)
    at module.exports (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/penthouse/lib/index.js:269:10)
    at /Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/critical/lib/core.js:170:16
    at tryCatcher (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:512:31)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._resolveCallback (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:432:57)
    at Promise._settlePromiseFromHandler (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:524:17)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:569:18)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:693:18)
    at Promise._fulfill (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:638:18)
    at Promise._resolveCallback (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:454:14)
    at ReductionPromiseArray._resolve (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/reduce.js:61:19)
    at ReductionPromiseArray._resolveEmptyArray (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/reduce.js:52:10)
    at ReductionPromiseArray.init (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise_array.js:71:18)
    at Promise._settlePromise (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:566:21)
    at Promise._settlePromise0 (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:614:10)
    at Promise._settlePromises (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/promise.js:693:18)
    at Async._drainQueue (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/async.js:133:16)
    at Async._drainQueues (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/async.js:143:10)
    at Immediate.Async.drainQueues [as _onImmediate] (/Users/owenbuckley/Workspace/pvdgeeks.org/website-frontend/node_modules/bluebird/js/release/async.js:17:14)
    at runCallback (timers.js:789:20)
    at tryOnImmediate (timers.js:751:5)
    at processImmediate [as _immediateCallback] (timers.js:722:5)
✨  Done in 21.65s.

and so just wanted to start the discussion to see if any of the breaking changes for plugins in v4 would apply here?
https://medium.com/webpack/webpack-4-migration-guide-for-plugins-loaders-20a79b927202

Maybe the removal of this.options? If help is needed, I can try and take a shot at it. Thanks for this great plugin! 🙏 ✌️

The text was updated successfully, but these errors were encountered:

anthonygore · 2018-03-10T05:00:19Z

Hey @thescientist13, sorry for the late reply. Have you made any progress on upgrading to v4?

thescientist13 · 2018-03-10T13:28:31Z

Hey @anthonygore ! I am still getting there piece by piece here but now that #10 has been merged (thank you!), I can just try updating webpack here in this repo and see if all the tests still pass. ✅

lili21 · 2018-03-10T16:43:04Z

it's extract-text-webpack-plugin's issue

bfkeinberg · 2018-03-20T20:49:46Z

It appears that for webpack v4 extract-text-webpack-plugin is dead and mini-css-extract-plugin is the paved road approach.

thescientist13 · 2018-03-21T13:30:57Z

@lili21 / @bfkeinberg
Yeah, that seems like the general consensus of the community from what I've been seeing. I think the big thing waiting from mini-css-extract-plugin (for me anyway at the time) was [contenthash] support (webpack-contrib/mini-css-extract-plugin#1) which seems to have landed.

@anthonygore
So, In the case of this plugin, since it only ever expected / assumed extract-text-webpack-plugin as a peerDependency, it seems like it is a safe bet to say that for the case of webpack v4 support, this plugin should steer in the direction of webpack itself, which is moving to native CSS bundling support. (mini-css-extract-plugin is only a stopgap to that eventuality).

I am going to test out mini-css-extract-plugin now in my side project and see it how it goes, mainly to make sure bundle sizes are comparable to that of extract-text-webpack-plugin. Will report back.

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

Seems to be working well for me with mini-css-extract-plugin based on my work in my project using webpack v4. (ProvidenceGeeks/website-frontend#142).

anthonygore · 2018-03-25T07:01:07Z

Yeah, I think you're right about mini-css-extract-plugin being the future. However, I think we proceed with extract-text-webpack-plugin for now, though, and wait and see where the dust settles. Do you agree?

thescientist13 · 2018-03-25T16:02:18Z

@anthonygore these comments seem to suggest ETWP will not be making the full jump to webpackv4 support.

My understanding is that mini-css-extract-plugin is the (official webpack team supported) way to go forward.

thescientist13 · 2018-03-28T15:41:31Z

So I think the issue may be related to Chrome Headless dependency. Switched the CircleCI build to use my personal Docker image that has Chrome Headless installed on my fork PR and now the build is passing

Maybe the dependency is coming from JSDOM? Let me see if I can dig further into where that dependency is coming from since I think it would ideal if we could get by without it due to the complex nature of setting up environments (local, CI, etc) to support it.

edit: seems to be coming from critical itself

$ npm ls puppeteer
[email protected] /Users/owenbuckley/Workspace/github/forks/html-critical-webpack-plugin
└─┬ [email protected]
  └─┬ [email protected]
    └── [email protected]

I guess the assumption being that if you want to run critical, you need to be on a host environment that expects chromium support?

This seems inline with my experiences using Chrome Headless in the context of Karma unit testing.

anthonygore · 2018-03-29T07:11:35Z

Interesting. Any ideas as to why this issue only happens when testing?

thescientist13 · 2018-04-02T14:38:32Z

not sure @anthonygore , but I will do some more testing today on this and hopefully get a straight answer as to what's the issue.

thescientist13 · 2018-04-02T17:46:23Z

FWIW I see similar output in my project too
https://circleci.com/gh/ProvidenceGeeks/website-frontend/327

(node:839) DeprecationWarning: Tapable.plugin is deprecated. Use new API on `.hooks` instead
(node:839) DeprecationWarning: Tapable.apply is deprecated. Call apply on the plugin directly instead
(node:839) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Failed to launch chrome!
/home/circleci/repo/node_modules/puppeteer/.local-chromium/linux-508693/chrome-linux/chrome: error while loading shared libraries: libXcomposite.so.1: cannot open shared object file: No such file or directory


TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md

(node:839) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

In this case, the build is not exiting too, which is similar to the behavior we're seeing here and so has probably been doing so for all my PRs.

So basically using critical as of version 1.0.0 is dependent on runtime environment that has support for Headless Chrome, and more to the point, the OS specific libraries needed to run it.

When this package was bumped to that version, my guess is that is when things started silently breaking for people 😞

And so in my PR above, it included a bump to this latest version of html-critical-webpack-plugin.

So next steps I see are basically around documentation / maintenance / tracking:

I ~~will~~ have opened an issue tracking this breaking change from critical in html-critical-webpack-plugin
I ~~will~~ have opened a PR to demonstrate outside of the unit testing context (using CircleCI to demonstrate)
We should merge that PR to add a CI step for this project
I rebase all this into my unit testing PR , which should then pass and do all the things
We can then release a new version of html-critical-webpack-plugin

Thoughts?

anthonygore · 2018-04-17T12:23:49Z

@thescientist13 was the PR meant for this repo? I think you've made it on your own repo 😃

thescientist13 · 2018-04-17T13:01:45Z

@anthonygore
My repo has CircleCI setup which I used to demonstrate the issue of the build failing, and then the build passing, when switching to a build time that has needed puppeteer operating system dependencies installed.

This aimed to make the case per my recommendation here to setup CI for this repo as a perquisite to any more (feature) work in general.

This was referenced Feb 25, 2018

upgrade to webpack v4 ProvidenceGeeks/website-frontend#142

Merged

added dependencies for unit testing and added specs #10

Merged

anthonygore mentioned this issue Mar 10, 2018

🐛 fix: specify css by default #6

Merged

thescientist13 mentioned this issue Mar 12, 2018

webpack v4 compatibility upgrade #12

Merged

3 tasks

thescientist13 mentioned this issue Mar 26, 2018

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

Closed

thescientist13 mentioned this issue Apr 2, 2018

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

Closed

anthonygore closed this as completed in #12 May 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

anticipated changes needed for webpack v4 support? #9

anticipated changes needed for webpack v4 support? #9

thescientist13 commented Feb 25, 2018

anthonygore commented Mar 10, 2018

thescientist13 commented Mar 10, 2018

lili21 commented Mar 10, 2018

bfkeinberg commented Mar 20, 2018

thescientist13 commented Mar 21, 2018

thescientist13 commented Mar 23, 2018 •

edited

Loading

anthonygore commented Mar 25, 2018

thescientist13 commented Mar 25, 2018 •

edited

Loading

thescientist13 commented Mar 28, 2018 •

edited

Loading

anthonygore commented Mar 29, 2018

thescientist13 commented Apr 2, 2018

thescientist13 commented Apr 2, 2018 •

edited

Loading

anthonygore commented Apr 17, 2018 •

edited

Loading

thescientist13 commented Apr 17, 2018 •

edited

Loading

anticipated changes needed for webpack v4 support? #9

anticipated changes needed for webpack v4 support? #9

Comments

thescientist13 commented Feb 25, 2018

anthonygore commented Mar 10, 2018

thescientist13 commented Mar 10, 2018

lili21 commented Mar 10, 2018

bfkeinberg commented Mar 20, 2018

thescientist13 commented Mar 21, 2018

thescientist13 commented Mar 23, 2018 • edited Loading

anthonygore commented Mar 25, 2018

thescientist13 commented Mar 25, 2018 • edited Loading

thescientist13 commented Mar 28, 2018 • edited Loading

anthonygore commented Mar 29, 2018

thescientist13 commented Apr 2, 2018

thescientist13 commented Apr 2, 2018 • edited Loading

anthonygore commented Apr 17, 2018 • edited Loading

thescientist13 commented Apr 17, 2018 • edited Loading

thescientist13 commented Mar 23, 2018 •

edited

Loading

thescientist13 commented Mar 25, 2018 •

edited

Loading

thescientist13 commented Mar 28, 2018 •

edited

Loading

thescientist13 commented Apr 2, 2018 •

edited

Loading

anthonygore commented Apr 17, 2018 •

edited

Loading

thescientist13 commented Apr 17, 2018 •

edited

Loading