extension: simplify bundle building #6282
Conversation
| /** @param {(status: [string, string, string]) => void} listenCallback */ | ||
| function listenForStatus(listenCallback) { | ||
| log.events.addListener('status', listenCallback); | ||
| } | ||
|
|
||
| if (typeof module !== 'undefined' && module.exports) { | ||
| // export for extension-entry to require (via browserify). | ||
| // export for require()ing (via browserify). | ||
| module.exports = { |
There was a problem hiding this comment.
since there's no unit tests for this bundle, it's not clear we actually want this module.exports block anymore. Do we? (e.g. more annoying to figure out later but minimal overhead to keep today)
There was a problem hiding this comment.
Yeah I'm not sure we need it either, but +1 to being paranoid for now :)
| @@ -180,23 +207,23 @@ if (typeof window !== 'undefined' && 'chrome' in window && chrome.runtime) { | |||
| } | |||
|
|
|||
| if (typeof module !== 'undefined' && module.exports) { | |||
| // Export for importing types into popup.js, require()ing into unit tests. | |||
| // Export for importing types into popup.js and require()ing into unit tests. | |||
There was a problem hiding this comment.
require()ing into unit tests is also a lie, but could be true someday :) Meanwhile this block is needed for the mentioned popup.js use
| @@ -1,37 +0,0 @@ | |||
| /** | |||
There was a problem hiding this comment.
had no effect on the output. Maybe it was required for some ancient version of speedline that pulled in catapult?
| return gulp.src('app/manifest.json') | ||
| .pipe(chromeManifest(manifestOpts)) |
There was a problem hiding this comment.
this may have come from the original generator. Not entirely sure what it's supposed to do, maybe copy over deps? But since the only target we give is the script bundle, and because we copy that ourselves below after browserifying it, this also has no effect but copying over the manifest file (which we can do more simply)
|
|
||
| // Inject the new browserified contents back into our gulp pipeline | ||
| file.contents = bundle.bundle(); | ||
| })) |
There was a problem hiding this comment.
popup.js doesn't require() any other files and hasn't in a long time, so no need to run it through browserify.
|
Aside from the entry file split, diffing a generated |
| @@ -27,7 +27,7 @@ const LR_PRESETS = { | |||
| * @return {Promise<string|Array<string>|void>} | |||
| */ | |||
| async function runLighthouseInLR(connection, url, flags, {lrDevice, categoryIDs, logAssets}) { | |||
| // Certain fixes need to kick-in under LR, see https://github.com/GoogleChrome/lighthouse/issues/5839 | |||
| // Certain fixes need to kick in under LR, see https://github.com/GoogleChrome/lighthouse/issues/5839 | |||
| .pipe(gulp.dest(distDir)); | ||
| }); | ||
|
|
||
| function applyBrowserifyTransforms(bundle) { | ||
| // Fix an issue with imported speedline code that doesn't brfs well. | ||
| return bundle.transform('./fs-transform', {global: true}) |
(we should probably expand the pr lint to include the other bundles besides
extension:)This is a simplification of the building of the bundles (mostly dt/extension).
Primary change is that
extension-entry.jsno longer relies on importingdevtools-entry.js, so all the entry points are independent of each other (they all just rely on core). There were two functionsextension-entry.jswas still depending on:getDefaultCategories()(which returns the categories in the default config)getDefaultConfigForCategories()(which returns a config that extends the default config but setsonlyCategoriesto the passed in categories)Turns out
getDefaultCategories()wasn't used indevtools-entry.jsanyways, so just moved it intoextension-entry.js.getDefaultConfigForCategories(), meanwhile, is so short and straightforward this just duplicates the function in both files.Things become a lot simpler to understand without the layering/pseudo-inheritance. Things are either set for the consumer of the file (e.g. on
selffor devtools) or exposed at the module level for unit testing.The other set of changes are a few simple fixes in the extension gulpfile, deleting stuff that wasn't actually doing anything.