scripts(i18n): support es modules in collect-strings

[connorjclark]

ref #12689

Confirmed working by merging with #12702.

I added a temporary file, just for review, tmp-esm-strings.js. That along with some tweaks to the script to ignore errors will be removed before merging, only exist to show the script works with ES modules now.

[connorjclark]

small change here... relativePath was not necessary.

[brendankenny]

• It might be worth planning the timing on these before starting? I'd really like to minimize the amount of time that our codebase is partially esm and partially commonjs. We definitely have some things to resolve (especially with bundling) before we can just do the whole thing in one fell swoop.
• Seems like there's two changes here? Supporting modules in collect-strings and converting collect-strings to using modules.
• there's another change to collect-strings that would love a review :)

[connorjclark]

• It might be worth planning the timing on these before starting? I'd really like to minimize the amount of time that our codebase is partially esm and partially commonjs. We definitely have some things to resolve (especially with bundling) before we can just do the whole thing in one fell swoop.

There needs to be some delay between starting and finishing since converting lighthouse-core requires a breaking change, and converting that + everything else in a reviewable, slow manner just as we are doing a major release is untenable, so I've been starting it now (converting the things that won't run afoul of our major version policy). At the moment, I've just been doing enough to land #12702 (collect-strings is a prerequisite).

(especially with bundling)

Are you referring to how we're going to replace browserifyFile with rollup-equivalent / how we'd get browserifyFile to process es modules? The latter seems most straightforward, but would require us to bring in babelify. EDIT: ok i just tried babelify and regeneratorRuntime reminded me how much i dislike babel, so nvm on that :)

Besides bundling, what else in on your mind?

• Seems like there's two changes here? Supporting modules in collect-strings and converting collect-strings to using modules.

Supporting ES modules is really just a two line change. Would a comment on the relevant lines suffice? Otherwise, I can remove from this PR (and use the created require in this script for now).

• there's another change to collect-strings that would love a review :)

done

[brendankenny]

we may need to make a style call on when to use inline export vs export blocks :)

[brendankenny]

It's not necessarily a breaking change (our primary interface is async already and conditional exports are a good match for us since we're not a utility you import lightly), but FWIW this is exactly the kind of discussion I meant we should have :)

I don't foresee us having a hard time converting our code. We purposefully don't push the boundaries of commonjs invariants, and where we have we've gotten burned in the past, so we do things like deep cloning even where not necessary :) Like these collect-strings files, I think the conversion of core files is going to largely be a mechanical change.

It's the tooling that is going to take more creative changes, and the code being in a mixture of module/commonjs states means a mixture of tooling states which is not going to be easy to reason about and it could hinder other work going on. All the different pieces aren't necessarily hard problems, but we don't really have a plan for some of them yet. I'd much rather we got all our ducks in a row, had a planned series of changes, then just had "esm week" (or days!) and knocked it all out at once.

• Seems like there's two changes here? Supporting modules in collect-strings and converting collect-strings to using modules.

Supporting ES modules is really just a two line change. Would a comment on the relevant lines suffice? Otherwise, I can remove from this PR (and use the created require in this script for now).

I mean, my preferred approach would be putting off the ES modules transition until it can be done at relatively the same time, so either switching to dynamic import() to support importing from util.js or require()ing from util-commonjs.js, since we're making that file specifically for transitional commonjs interop.

[connorjclark]

I mean, my preferred approach would be putting off the ES modules transition until it can be done at relatively the same time, so either switching to dynamic import() to support importing from util.js

word... i keep forgetting you can import from commonjs. updated pr.

[connorjclark]

@adamraine ready for review