-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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.
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
Embedding in another extension #7350
Comments
I'm not sure what exactly you're feature requesting from us. You can use the browserified extension script we already make for our extension today. |
@patrickhulce so, I ended up looking into this and doing something like: const lighthouse = require('./node_modules/lighthouse/lighthouse-core/index');
const ExtensionProtocol = require('./node_modules/lighthouse/lighthouse-core/gather/connections/extension');
const defaultConfig = require('./node_modules/lighthouse-core/config/default-config')
const Config = require('./node_modules/lighthouse-core/config/config');
const i18n = require('./node_modules/lighthouse-core/lib/i18n/i18n.js'); And then creating a new I'll look into using the browserified extension script - is that available as an artifact in the npm module? If it is - is there an API I can access? update: it looks like by building the extension I can't require it in other JavaScript very easily, I can however access the globals exported by: window.runLighthouseInExtension=runLighthouseInExtension;
window.getDefaultCategories=getDefaultCategories;
window.isRunning=isRunning;
window.listenForStatus=listenForStatus;
window.loadSettings=loadSettings;
window.saveSettings=saveSettings; which is a start but not very customisable or great since it doesn't let me configure things very well (runLighthouseInExtension for example doesn't take a url but uses the current one in the active tab). |
Edit, the custom build of the browserified extension script isn't too fun to use either. Basically what I'm requesting is an entry point I can |
So, doing the first idea (not using the bundle) isn't very straightforward, lighthouse requires pretty custom builds (with brfs) and getting it to work with our webpack build is pretty frustrating (brfs in webpack through transform loader didn't work well). Currently trying to tweak the options in build-bundle with the |
Would you be open to a PR adding a flag that exports an option to take a tabId and URL as a property exported from I'm willing to contribute this if you'd take it. |
Use @brendankenny's comment, it's much better 😄 If you're super curious about my thoughts, you can check the edit history. |
hi @benjamingr! If I understand what you're trying to do, I think this is possible, yeah.
I'm thinking something like this // known-tab-extension-connection.js
const ExtensionConnection = require('./lighthouse-core/gather/connections/extension.js');
class KnownTabExtensionConnection extends ExtensionConnection {
/** @param {number} currentTab */
constructor(currentTab) {
super();
this._currentTab = currentTab;
}
/** @return {Promise<chrome.tabs.Tab>} */
_queryCurrentTab() {
return new Promise((resolve, reject) => {
chrome.tabs.get(this._currentTab, tab => {
if (chrome.runtime.lastError) {
return reject(chrome.runtime.lastError);
}
resolve(tab);
});
});
}
} and then for the entry point // known-tab-extension-entry.js
const lighthouse = require('./lighthouse-core/index.js');
const KnownTabExtensionConnection = require('./known-tab-extension-connection.js');
const defaultConfig = require('./lighthouse-core/config/default-config.js');
/**
* @param {number} knownTabId
* @param {LH.Flags} flags Lighthouse flags.
* @return {Promise<LH.Result>}
*/
async function runLighthouse(knownTabId, flags) {
const connection = new KnownTabExtensionConnection(knownTabId);
const url = await connection.getCurrentTabURL();
const runnerResult = await lighthouse(url, flags, config, connection);
if (!runnerResult) throw new Error('did you run Lighthouse in gatherMode?');
return runnerResult.lhr;
}
// module and/or window export of runLighthouse() Running Some issues remain based on the particulars of what you end up needing.
|
Thanks! I actually did get this to work by keeping the existing build, editing runLighthouseInExtension in the source and passing it the tabId manually for the connection. I actually started with just using lighthouse-core and ExtensionConnetion but that ended up being hard because of the particular transforms and build lighthouse uses for building for extensions (like brfs, ignoring a bunch of folders etc). I talked to @pkrumins and he told me we can do something like adding a third build option to lighthouse that sets Alternatively - we can add a build flag that doesn't export all the current window globals but instead exports something an extension can use like a method taking a tabId, url and what transforms to use and runs lighthouse. We can export that to NPM and have that consumable by extensions - what do you think about that option? It'd really be great not having to keep a fork of the code for this and using lighthouse as an npm module for browser extension development. |
Rather than publishing another npm module (with the maintenance and ownership overhead that brings), I think maybe the effort could be put into making the use of lighthouse as an npm module in this way easier. A Then it sounds like adding a |
maybe we could even always do We'd have to see if that would be a net improvement in code readability or if it would just make things more complicated than it's worth, though. |
That would be awesome @brendankenny |
The extension has been increasingly difficult to maintain due to changes in the protocol, limitations of the protocol through the extension APIs, and increased out-of-process use in Chrome, so we've decided to remove the actual run of Lighthouse in the extension. Instead, users will be pointed to PSI and DevTools. As a result, unfortunately we won't be moving forward with this and will be removing extension-specific code. See #9193 for progress on that. |
I am confused - what was hard about maintaining extensions in the devtools (the protocol rarely changes nowadays anyway?). Also - what do you mean by PSI? does that mean users will be referred to a google hosted service and that the devtools will send site content to google servers rather than run audits locally? Or do you mean users will still be able to run audits locally but that will require node and PSI will be offered as a free alternative for users who do not want to run node? |
See discussion in #8690 and things like the saga of trying to get OOPIF support in #7517, #7566, and finally #7608.
Exactly this. The extension will offer PSI as an option for users still wanting a one-click Lighthouse run (with clear indication that it uses an API for the service) as well as offering the option of opening DevTools and running it there. DevTools and the node versions of Lighthouse will not be changed, still run completely locally. |
Feature request summary
Hey, I think it could be really nice if there was a way to embed lighthouse in another extension.
We're developing an extension that already utilizes the devtools protocol and opens chrome.debugger.
Currently I was thinking about embedding an audit through the programmatic interface and CLI tool but it would be much nicer if there was a way to (from a Chrome extension context) pass a connected
chrome.debugger
tabId (or something similar) and embed lighthouse into another extension.What is the motivation or use case for changing this?
Improving the UI/UX of the extension and being able to use lighthouse directly instead of reusing parts of it or giving users a subpar solution requiring a cli tool.
How is this beneficial to Lighthouse?
Bigger/better ecosystem and motivation for more extensions to get into the "care about site performance" field.
The text was updated successfully, but these errors were encountered: