tests(smokehouse): convert to ES modules #13046
Conversation
connorjclark
requested review from
patrickhulce
and removed request for
a team
lighthouse-cli/test/smokehouse/test-definitions/mixed-content/expectations.js
Outdated
Show resolved
Hide resolved
| const lighthouse = require('../../fraggle-rock/api.js'); | ||
| const puppeteer = require('puppeteer'); | ||
| const StaticServer = require('../../../lighthouse-cli/test/fixtures/static-server.js').Server; | ||
| // TODO(esmodules): Node 14, 16 crash with `--experimental-vm-modules` if require and import |
There was a problem hiding this comment.
nodejs/node#39330 is back :(
lighthouse-cli/test/smokehouse/test-definitions/forms/form-config.js
Outdated
Show resolved
Hide resolved
lighthouse-cli/test/smokehouse/test-definitions/mixed-content/expectations.js
Outdated
Show resolved
Hide resolved
package.json
Outdated
| @@ -21,7 +21,7 @@ | |||
| "build-extension-firefox": "node ./build/build-extension.js firefox", | |||
| "build-devtools": "yarn reset-link && node ./build/build-bundle.js clients/devtools-entry.js dist/lighthouse-dt-bundle.js && node ./build/build-dt-report-resources.js", | |||
| "build-smokehouse-bundle": "node ./build/build-smokehouse-bundle.js", | |||
| "build-lr": "yarn reset-link && node ./build/build-lightrider-bundles.js", | |||
| "build-lr": "yarn reset-link && node ./build/build-lightrider-bundles.js && rollup lighthouse-cli/test/fixtures/static-server.js -o dist/lightrider/static-server.js -p commonjs -p node-resolve -e mime-db,glob", | |||
There was a problem hiding this comment.
Updates build-lr to also bundle up static-server.js, necessary since it now imports non-core modules (And bundling it all up is simplest way to run it in google3).
Can you explain what changed (those deps were added two years ago)? And maybe just for my lack of rollup knowledge: why mime-db and not mime-types?
There was a problem hiding this comment.
why mime-db and not mime-types?
Should be mime-types. I chose the wrong package name.
es-main was added, which is not in google3. mime-types and glob are in google3, so I made them externals here to keep the bundle smaller.
Also, due to how we currently roll the static server into google3 (copy/paste, basically), the ../../../root.js import is left as-is (which wouldn't work), and the code is string replaced to turn the import into a const. Since the file is now bundled, rollup takes care of that. Future imports form outside this file should be handled without further complication now that rollup is being used (sans an optional step of "is this dep in google3? ok, let's just make it external"
There was a problem hiding this comment.
And due to overlooking while splitting out this PR, I forgot to make this file esm and actually use es-main.
There was a problem hiding this comment.
so I made them externals here to keep the bundle smaller.
Ah, got it, thanks. This was the part I was missing
There was a problem hiding this comment.
should I make a build-lh.sh script so I can leave this comment?
I prefer using rollup CLI where possible as it's pretty simple to read and write.
There was a problem hiding this comment.
maybe it's fine? I should have looked for the -e flag but got distracted by the node-resolve plugin instead. At the very least blame will find this conversation :)
ref #13045