Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc(build): bundle pub ads plugin for devtools #9924

Merged
merged 10 commits into from
Jan 28, 2020
Merged

misc(build): bundle pub ads plugin for devtools #9924

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
948ee09
WIP: include plugin in dt build
brendankenny Nov 5, 2019
082715b
feedback
brendankenny Nov 6, 2019
55839fd
bundling pubads for devtools (#9936)
connorjclark Nov 9, 2019
b6f347a
prepare to land
connorjclark Nov 9, 2019
85dbdfd
comment
connorjclark Nov 9, 2019
e2fa729
link
connorjclark Nov 9, 2019
fc482d1
bundlesize
connorjclark Nov 9, 2019
4e47886
Merge remote-tracking branch 'origin/master' into dtplugin
connorjclark Jan 27, 2020
f24f424
rm tsignore
connorjclark Jan 27, 2020
972191f
bundlesize bump
paulirish Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions build/build-bundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ const gatherers = LighthouseRunner.getGathererList()
const locales = fs.readdirSync(__dirname + '/../lighthouse-core/lib/i18n/locales/')
.map(f => require.resolve(`../lighthouse-core/lib/i18n/locales/${f}`));

// HACK: manually include the lighthouse-plugin-publisher-ads audits.
/** @type {Array<string>} */
// @ts-ignore
const pubAdsAudits = require('lighthouse-plugin-publisher-ads/plugin.js').audits.map(a => a.path);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or whatever is a good way to get this list...readdir doesn't work cause there's more than just the audits in that directory


/** @param {string} file */
const isDevtools = file => path.basename(file).includes('devtools');
/** @param {string} file */
Expand Down Expand Up @@ -84,6 +89,7 @@ async function browserifyFile(entryPath, distPath) {
}

// Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded.
// Exposed path must be a relative path from lighthouse-core/config/config-helpers.js (where loading occurs).
const corePath = './lighthouse-core/';
const driverPath = `${corePath}gather/`;
audits.forEach(audit => {
Expand All @@ -93,6 +99,15 @@ async function browserifyFile(entryPath, distPath) {
bundle = bundle.require(gatherer, {expose: gatherer.replace(driverPath, '../gather/')});
});

// HACK: manually include the lighthouse-plugin-publisher-ads audits.
// TODO: there should be a test for this.
if (isDevtools(entryPath)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinda feels like we should resolve those functions to booleans called isDevtools and call the functions isDevtoolsEntry or something

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kinda feels like we should resolve those functions to booleans called isDevtools and call the functions isDevtoolsEntry or something

yeah, I had a similar thought. Not sure why they're like that...I assume it made sense as a one-off and we've just c/p it multiple times now

bundle.require('lighthouse-plugin-publisher-ads');
pubAdsAudits.forEach(pubAdAudit => {
bundle = bundle.require(pubAdAudit);
});
}

// browerify's url shim doesn't work with .URL in node_modules,
// and within robots-parser, it does `var URL = require('url').URL`, so we expose our own.
// @see https://github.com/GoogleChrome/lighthouse/issues/5273
Expand Down
7 changes: 6 additions & 1 deletion clients/devtools-entry.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ const {registerLocaleData, lookupLocale} = require('../lighthouse-core/lib/i18n/

/**
* Return a version of the default config, filtered to only run the specified
* categories.
* categories. If `lighthouse-plugin-publisher-ads` is in the list of
* `categoryIDs` the plugin will also be run.
* @param {Array<string>} categoryIDs
* @return {LH.Config.Json}
*/
function getDefaultConfigForCategories(categoryIDs) {
return {
extends: 'lighthouse:default',
plugins: ['lighthouse-plugin-publisher-ads'],
settings: {
onlyCategories: categoryIDs,
},
Expand Down Expand Up @@ -53,6 +55,9 @@ if (typeof module !== 'undefined' && module.exports) {
// Expose only in DevTools' worker
// @ts-ignore
if (typeof self !== 'undefined') {
// TODO: refactor and delete `global.isDevtools`.
global.isDevtools = true;

// @ts-ignore
self.setUpWorkerConnection = setUpWorkerConnection;
// @ts-ignore
Expand Down
12 changes: 10 additions & 2 deletions lighthouse-core/config/config-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,16 @@ function requireAudits(audits, configDir) {
const coreAudit = coreList.find(a => a === auditPathJs);
let requirePath = `../audits/${audit.path}`;
if (!coreAudit) {
// Otherwise, attempt to find it elsewhere. This throws if not found.
requirePath = resolveModule(audit.path, configDir, 'audit');
// TODO: refactor and delete `global.isDevtools`.
if (global.isDevtools) {
// This is for pubads bundling.
requirePath = audit.path;
} else {
// Otherwise, attempt to find it elsewhere. This throws if not found.
const absolutePath = resolveModule(audit.path, configDir, 'audit');
// Use a relative path so bundler can easily expose it.
requirePath = path.relative(__dirname, absolutePath);
}
}
implementation = /** @type {typeof Audit} */ (require(requirePath));
}
Expand Down
5 changes: 4 additions & 1 deletion lighthouse-core/config/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,10 @@ class Config {
for (const pluginName of pluginNames) {
assertValidPluginName(configJSON, pluginName);

const pluginPath = resolveModule(pluginName, configDir, 'plugin');
// TODO: refactor and delete `global.isDevtools`.
const pluginPath = global.isDevtools ?
pluginName :
resolveModule(pluginName, configDir, 'plugin');
const rawPluginJson = require(pluginPath);
const pluginJson = ConfigPlugin.parsePlugin(rawPluginJson, pluginName);

Expand Down
5 changes: 3 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"build-all:task": "(yarn build-extension & yarn build-devtools & yarn build-lr & yarn build-viewer & wait) && yarn build-pack",
"build-all:task:windows": "yarn build-extension && yarn build-devtools && yarn build-lr && yarn build-viewer",
"build-extension": "node ./build/build-extension.js",
"build-devtools": "node ./build/build-bundle.js clients/devtools-entry.js dist/lighthouse-dt-bundle.js && node ./build/build-dt-report-resources.js",
"build-devtools": "yarn link && yarn link lighthouse && node ./build/build-bundle.js clients/devtools-entry.js dist/lighthouse-dt-bundle.js && node ./build/build-dt-report-resources.js",
"build-lr": "node ./build/build-lightrider-bundles.js",
"build-pack": "bash build/build-pack.sh",
"build-viewer": "node ./build/build-viewer.js",
Expand Down Expand Up @@ -121,6 +121,7 @@
"isomorphic-fetch": "^2.2.1",
"jest": "^24.9.0",
"jsdom": "^12.2.0",
"lighthouse-plugin-publisher-ads": "^0.4.1",
"npm-run-posix-or-windows": "^2.0.2",
"nyc": "^13.3.0",
"package-json-versionify": "^1.0.4",
Expand Down Expand Up @@ -189,7 +190,7 @@
},
{
"path": "./dist/lighthouse-dt-bundle.js",
"maxSize": "400 kB"
"maxSize": "410 kB"
},
{
"path": "./dist/lightrider/report-generator-bundle.js",
Expand Down
1 change: 1 addition & 0 deletions types/node.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

declare module NodeJS {
interface Global {
isDevtools?: boolean;
isLightrider?: boolean;
}
}
45 changes: 43 additions & 2 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,11 @@
resolved "https://registry.yarnpkg.com/@nodelib/fs.stat/-/fs.stat-1.1.2.tgz#54c5a964462be3d4d78af631363c18d6fa91ac26"
integrity sha512-yprFYuno9FtNsSHVlSWd+nRlmGoAbqbeCwOryP6sC/zoCjhpArcRMYp19EvpSUSizJAlsXEwJv+wcWS9XaXdMw==

"@tusbar/cache-control@^0.3.1":
version "0.3.1"
resolved "https://registry.yarnpkg.com/@tusbar/cache-control/-/cache-control-0.3.1.tgz#2ee673c6a7166041b5d419f7e15cd9f16e21c8e1"
integrity sha512-qeQhWoHQqDdW+SS1TgybdOeaSo+8tZx4ZesMY0+HSLeWWa2bGDetB9wxAT/umlXuutC6QdbADd48RJ6hrYr4BQ==

"@types/archiver@^2.1.2":
version "2.1.2"
resolved "https://registry.yarnpkg.com/@types/archiver/-/archiver-2.1.2.tgz#e84960d4872570d7c826589cd57f2c076bf198c5"
Expand Down Expand Up @@ -513,6 +518,13 @@
dependencies:
"@types/node" "*"

"@types/intl-messageformat@^3.0.0":
version "3.0.0"
resolved "https://registry.yarnpkg.com/@types/intl-messageformat/-/intl-messageformat-3.0.0.tgz#a405302abfa55c427cbcd374cecb12c0b90e6915"
integrity sha512-sOzBp/B4GlQZnh1ZHCJ/vVoIkhaErXHVQJnFE0P7LtEcdaIy5qCMMevXy5aOu73VWzZQ5MCv2PdOTFOpEDLQhw==
dependencies:
intl-messageformat "*"

"@types/istanbul-lib-coverage@*", "@types/istanbul-lib-coverage@^2.0.0":
version "2.0.1"
resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-2.0.1.tgz#42995b446db9a48a11a07ec083499a860e9138ff"
Expand Down Expand Up @@ -2957,7 +2969,7 @@ esprima@^3.1.3:
resolved "https://registry.yarnpkg.com/esprima/-/esprima-3.1.3.tgz#fdca51cee6133895e3c88d535ce49dbff62a4633"
integrity sha1-/cpRzuYTOJXjyI1TXOSdv/YqRjM=

esprima@^4.0.0:
esprima@^4.0.0, esprima@^4.0.1:
version "4.0.1"
resolved "https://registry.yarnpkg.com/esprima/-/esprima-4.0.1.tgz#13b04cdb3e6c5d19df91ab6987a8695619b0aa71"
integrity sha512-eGuFFw7Upda+g4p+QHvnW0RyTX/SVeJBDM/gCtMARO0cLuT2HcEKnTPvhjV6aGeqrCB/sbNop0Kszm0jsaWU4A==
Expand Down Expand Up @@ -3990,12 +4002,30 @@ insert-module-globals@^7.0.0:
undeclared-identifiers "^1.1.2"
xtend "^4.0.0"

intl-format-cache@^4.2.5:
version "4.2.5"
resolved "https://registry.yarnpkg.com/intl-format-cache/-/intl-format-cache-4.2.5.tgz#6319f5707cdc766ab5c196990a6e55b0df9cc70f"
integrity sha512-xvvog/4HTVhJIg5dexcs6Ji/ROlolCgtz3std23bLEmucoGPrUVrYQPTcBWR314NM9khm4JSrdOamv9SEtvCUg==

intl-messageformat-parser@^1.8.1:
version "1.8.1"
resolved "https://registry.yarnpkg.com/intl-messageformat-parser/-/intl-messageformat-parser-1.8.1.tgz#0eb14c5618333be4c95c409457b66c8c33ddcc01"
integrity sha512-IMSCKVf0USrM/959vj3xac7s8f87sc+80Y/ipBzdKy4ifBv5Gsj2tZ41EAaURVg01QU71fYr77uA8Meh6kELbg==

intl-messageformat@^4.4.0:
intl-messageformat-parser@^3.2.2:
version "3.2.2"
resolved "https://registry.yarnpkg.com/intl-messageformat-parser/-/intl-messageformat-parser-3.2.2.tgz#28cacc80a53823a28b5ecaf16b3b35d0a1d5f9d2"
integrity sha512-ax9639ST77YimAddTH/0Q9qhXuYS8ZVsoqOZinqnS90MbXaNuIq+KIdifaIndwI+lMEv3o+qNaGycXYvlh17rw==

intl-messageformat@*:
version "7.5.0"
resolved "https://registry.yarnpkg.com/intl-messageformat/-/intl-messageformat-7.5.0.tgz#53582b6e911d4f41434e9d897b4789d2a2b2ff8e"
integrity sha512-s08OFUbRVUPTiXS8OOP06OhfehM1K4ughymhOazY4OhQMO+RiY4bnmsqRgYe7XorETg7mZnwJX1M34sJrzKoOA==
dependencies:
intl-format-cache "^4.2.5"
intl-messageformat-parser "^3.2.2"

intl-messageformat@^4.1.2, intl-messageformat@^4.4.0:
version "4.4.0"
resolved "https://registry.yarnpkg.com/intl-messageformat/-/intl-messageformat-4.4.0.tgz#aa196a4d04b573f4090bc417f982d81de4f74fad"
integrity sha512-z+Bj2rS3LZSYU4+sNitdHrwnBhr0wO80ZJSW8EzKDBowwUe3Q/UsvgCGjrwa+HPzoGCLEb9HAjfJgo4j2Sac8w==
Expand Down Expand Up @@ -5080,6 +5110,17 @@ lighthouse-logger@^1.0.0, lighthouse-logger@^1.2.0:
debug "^2.6.8"
marky "^1.2.0"

lighthouse-plugin-publisher-ads@^0.4.1:
version "0.4.1"
resolved "https://registry.yarnpkg.com/lighthouse-plugin-publisher-ads/-/lighthouse-plugin-publisher-ads-0.4.1.tgz#745aa00d691d329fcf3ed58ec5ef1e4cd0798e71"
integrity sha512-ksq9VMoHS3oH4naCGm6x0oCHnm7uMvAxcQpB57wZbJUQpPNmfeXRXwKuTyAei+TyO4HzVMkhCzuYzvVRcXpYdQ==
dependencies:
"@tusbar/cache-control" "^0.3.1"
"@types/intl-messageformat" "^3.0.0"
esprima "^4.0.1"
intl-messageformat "^4.1.2"
yargs "^13.3.0"

load-json-file@^1.0.0:
version "1.1.0"
resolved "https://registry.yarnpkg.com/load-json-file/-/load-json-file-1.1.0.tgz#956905708d58b4bab4c2261b04f59f31c99374c0"
Expand Down