deps: use esbuild-plugins-node-modules-polyfill

[MichaelDeBoey]

This PR replaces @esbuild-plugins/node-modules-polyfill with the up-to-date & maintained esbuild-plugins-node-modules-polyfill

The esbuild-plugins repo itself points towards using esbuild-plugin-polyfill-node instead
https://github.com/remorses/esbuild-plugins/blob/373b44902ad3e669f7359c857de09a930ce1ce90/README.md?plain=1#L15-L16

After doing this in the Remix repo (see remix-run/remix#5274), we got quite some new bugs, so we chose to go for @imranbarbhuiya's esbuild-plugins-node-modules-polyfill instead (see remix-run/remix#6562), which is an up-to-date and well maintained alternative

Added benefit is that we won't get the following deprecation warnings when installing @esbuild-plugins/node-modules-polyfill:

npm WARN deprecated rollup-plugin-inject@3.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-inject.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead

[adamraine]

Interesting, let's see if our CI jobs work. @connorjclark any thoughts?

Edit: yeah it's not a drop in replacement

[adamraine]

Could you add this to the esbuild-plugins.js file?

[MichaelDeBoey]

It should be a drop in replacement normally 🤔

Added this line to esbuild-plugins.js as well as requested

[connorjclark]

The failing tests show two critical issues:

1. zlib inflate code is not being removed. Maybe the zlib polyfil is no longer split into __zlib-lib/inflate and __zlib-lib/deflate modules, which would explain that. This is the only error CI shows, doesn't get to running the bundled tests.
2. more importantly, the bundled smoke tests fail totally. Comment out that zlib check throwing in CI, and then run DEBUG=1 yarn build-devtools and then yarn test-bundle seo you'll see:

Error: Worker returned an error: ReferenceError: navigator is not defined
Log:
[STDERR] ReferenceError: navigator is not defined
    at node-modules-polyfills:process (file:///Users/cjamcl/src/lighthouse/dist/node-modules-polyfills:process:63:9)
    at __init (file:///Users/cjamcl/src/lighthouse/dist/lighthouse-dt-bundle.js:20:58)
    at build/process-global.js (file:///Users/cjamcl/src/lighthouse/build/process-global.js:1:1)
    at __init (file:///Users/cjamcl/src/lighthouse/dist/lighthouse-dt-bundle.js:20:58)
    at file:///Users/cjamcl/src/lighthouse/clients/devtools/devtools-entry.js:1:1
    at file:///Users/cjamcl/src/lighthouse/dist/lighthouse-dt-bundle.js:93839:3
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

This error seems very non-trivial, and I'm not sure where to begin to fix it. The current setup works with no issues for us so I don't think there's any urgent motivation for us to change right now.

[MichaelDeBoey]

I think @imranbarbhuiya can probably help out with fixing all these errors

[adamraine]

Please let us know if get this to work, as we are interested in an alternative solution. For now we will close this PR

[imranbarbhuiya]

@adamraine I checked the file

lighthouse/build/build-bundle.js

Line 111 in f33cf93

'__zlib-lib/inflate': `

and saw you are adding some empty functions for __zlib/inflate so can I know why it's needed? Are you using this polyfill anywhere in your code? If yes, why change it to empty export? maybe the empty option in modules can fix the issue? I'll play around with these options and will let you know

[connorjclark]

The intention is to exclude the large amount of code needed to decompress data. Our library only needs to compress. The empty functions being exported remain to prevent some other part of the esbuild process from erroring during bundle.

[imranbarbhuiya]

@connorjclark I checked the previous code and saw it was using a different polyfill lib than the one used in @esbuild-plugins/node-modules-polyfill so I checked that repo and found that it doesn't use pako directly but uses its code in a similar folder structure. So I changed the shimObj keys to match pako's file structure but it's still throwing. I even added all the files where inflate_fast is used/defined in https://github.com/imranbarbhuiya/lighthouse/blob/37ed90e5f95bc9bc3d3308838956a67575d54d1a/build/build-bundle.js#L110-L115 but still same. Since you patched this change in #15239 maybe you can help with this?

just realized we use nodelibs folder of jspm and not src-browser where pako is used. So jspm is bundling everything in 1 file. And since esbuild doesn't removes unused exports, so the above approach wont work. I'll create an issue in jspm to ask them to split these long files if possible