clients(lr): export primary api, presets from lr bundle #14425
Conversation
| @@ -163,4 +163,7 @@ if (typeof window !== 'undefined') { | |||
|
|
|||
| export { | |||
| runLighthouseInLR, | |||
| lighthouse, | |||
There was a problem hiding this comment.
can the above manual window assignment be removed? does umd bundle assign to global for us in browser context?
There was a problem hiding this comment.
the window assignment can be removed if we switch our LR client from window.runLighthouseInLR to window.lightriderBundle.runLighthouseInLR.
(aka: doable)
There was a problem hiding this comment.
name is necessary for umd, and also is used to namespace on the global. makes sense.
That seems like a decent change to make IMO. Although, I would just name it lightrider (or lighthouse?)
build/build-bundle.js
Outdated
| @@ -75,7 +75,7 @@ const banner = ` | |||
| * @param {{minify: boolean}=} opts | |||
| * @return {Promise<void>} | |||
| */ | |||
| async function buildBundle(entryPath, distPath, opts = {minify: true}) { | |||
| async function buildLrBundle(entryPath, distPath, opts = {minify: true}) { | |||
There was a problem hiding this comment.
This function is also used to bundle CDT via the CLI. I think you need to add to opts a format that defaults to iife.
There was a problem hiding this comment.
yupppppppp okay i see it now.
since the use wasn't picked up by TS i missed it.
roger that, i'll make it extensible.
There was a problem hiding this comment.
that's fair, took me a second to grok why this wasn't ok b/c of that too
There was a problem hiding this comment.
since this CL is getting a lil bit more involved... i could also switch devtools to adopt the UMD pattern as well.. wdyt?
it'd mean we get to drop all of these in the devtools-entry:
// @ts-expect-error
self.X = X;There was a problem hiding this comment.
sgtm. You'll probably need to disable the devtool CI check first? Might be weird given you can't roll latest to CDT just yet. Maybe you'd just need to cherry pick into branch-9 and roll from there to get CI to pass.
build/build-bundle.js
Outdated
| @@ -231,7 +231,8 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) { | |||
| await bundle.write({ | |||
| file: distPath, | |||
| banner, | |||
| format: 'iife', | |||
| format: 'umd', | |||
| name: 'lightriderBundle', | |||
There was a problem hiding this comment.
remove name / make an optional param if you need it for LR bundle
paulirish
changed the title
This change allows g3 clients to consume the LR bundle as a CJS module, in addition to our existing LR use where we consume the globals.Nov 16 update: I've changed the intent of this branch. While doing a UMD build for LR and DevTools might still be nice, the primary item is to expose the full api so the FR modes are available.