Calling `critical.generate` lots of times in parallel seems to cause MaxListenersExceededWarning

[gregives]

I am the author of the eleventy-critical-css Eleventy plugin which calls critical.generate for every HTML file outputted by Eleventy. An issue was filed on eleventy-critical-css saying that the plugin was causing a MaxListenersExceededWarning.

When the plugin is used with larger Eleventy sites, critical.generate is called lots of times in parallel and it causes a MaxListenersExceededWarning. Here is the stack trace:

(node:12968) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 SIGINT listeners added to [process]. Use emitter.setMaxListeners() to increase limit      
    at _addListener (events.js:475:17)
    at process.addListener (events.js:497:10)
    at module.exports (D:\Projects\critical-issue-528-reproduction\node_modules\penthouse\lib\index.js:160:11)
    at D:\Projects\critical-issue-528-reproduction\node_modules\critical\src\core.js:75:20
    at D:\Projects\critical-issue-528-reproduction\node_modules\p-all\index.js:4:67
    at D:\Projects\critical-issue-528-reproduction\node_modules\p-map\index.js:57:28

This repository was added to the issue on eleventy-critical-css which successfully reproduces the problem. If I have time, I will create a simpler reproduction, probably just by calling critical.generate lots of times in parallel. See new simpler repository below.

I am unsure if this is a problem with Critical or with Penthouse, let me know if you want me to raise an issue in the Penthouse repository instead. Thanks for your help!

[gregives]

I have created a simpler repository to reproduce this issue: https://github.com/gregives/critical-issue-528-reproduction

[bezoerb]

Thanks @gregives. I'll take a look if this has anything todo with critical.

@pocketjoso what do you think? Have you ever encountered this warning in penthouse?

[bezoerb]

hey @gregives, sorry for the late reply. I looked into this but i don't know if i can do anything about it on this side. I could try to implement something like this example for critical but this would only work if you could provide a list of all urls at once. I don't know if this is possible because i have no experience with eleventy.

It should also be possible to implement this directly in your plugin. I hacked around a bit inside node_modules/eleventy-critical-css/index.js to test if this works (very rough though, just as a prove of concept ;)) https://gist.github.com/bezoerb/c88576602636606cc771da990fa3ef0a

Looks like this does the trick already

[gregives]

Hey @bezoerb, thank you so much for looking into this, no need to apologise for the late reply!

I've just taken a look at the Critical codebase and I think it might be possible to implement without a list of all URLs, would you be open to a pull request? I'll give it a go and if not, I'll make a change to my plugin. Thank you for your Gist, that's really helpful!

[bezoerb]

Hey @gregives I'm curious how you would solve this without knowing the urls upfront. If it works out it would be a nice addition to critical so feel free to draft a PR. I'm happy to review ;)

[gregives]

Hey @bezoerb, I hope you're well!

I finally got around to fixing this warning in eleventy-critical-css, here's my commit that fixes it: https://github.com/gregives/eleventy-critical-css/commit/a45d2b3917443546e2bd232bd77752442532e021#diff-e727e4bdf3657fd1d798edcd6b099d6e092f8573cba266154583a746bba0f346

I believe the fix could be applied to critical itself without much work, by doing something similar to the startProcessing and processFunction functions. It works without knowing a list of URLs upfront like we talked about above.

I don't have much spare time at the moment, but if I get a minute then I'll raise a PR for it!