abhinaba-ghosh / playwright-lighthouse

:performing_arts:: Playwright Lighthouse Audit
https://www.npmjs.com/package/playwright-lighthouse
MIT License
240 stars 28 forks source link

Use fraggle rock API for conducting lighthouse tests #44

Open asambstack opened 1 year ago

asambstack commented 1 year ago

In the legacy lighthouse flow, a browser needs to be started with remote debugging port enabled and lighthouse starts testing by firing CDP commands over this RDP. Lighthouse team has been working on the fraggle rock API with flow support and several API changes (see: https://github.com/GoogleChrome/lighthouse/issues/11313). This also includes a navigation API that can connect to existing puppeteer page instance for performing lighthouse tests. By modifying the playwright object slightly, we can utilise the fraggle rock API for performing lighthouse tests.

Pros

Changes

asambstack commented 1 year ago

Sure, was currently updating the readme with examples. Will also attach relevant links that were used as references in this PR. Currently, lighthouse team themselves haven't created docs for the same. However details of the fraggle rock implementation can be found in https://docs.google.com/document/d/1fRCh_NVK82YmIi1Zq8y73_p79d-FdnKsvaxMy0xIpNw/edit?pli=1#heading=h.fpighughqndf

asambstack commented 1 year ago

Hey @abhinaba-ghosh Have updated the tests, README and type definition. Could you review it once?

abhinaba-ghosh commented 1 year ago

I will be testing this today and release a new version.

abhinaba-ghosh commented 1 year ago

Hey @asambstack sorry for the delay. Could you rebase your branch and resolve the conflicts?

Xotabu4 commented 1 year ago

@asambstack any updates on this? Really looking to get this feature into package!

asambstack commented 1 year ago

Hey @abhinaba-ghosh Looks like in the newer package version we've released lighthouse 10 support. In lighthouse 10, the fraggle rock API feature has been released with the core package and hence we do not need to use the fraggle rock file imports specifically and can directly use the core lighthouse package itself.. However, came across an issue with using this approach with playwright, it was failing on some sites like nykaa.com. On deeper investigation, looks like we might need to do more work for monkey patching playwright page object to seem like that of puppeteer.

The way lighthouse works with a puppeteer page is that it first creates a session object that acts as a wrapper around a puppeteer session. One of the things that lighthouse does with a puppeteer page is to attach event listeners to it (ref). This is possible due to the fact that puppeteer exposes a connection method to the CDP session object it has created (ref) which is an extension of an event emitter. This connection class exposed by puppeteer emits different events whenever the state of the webpage changes, for eg. Target.attachedToTarget / Target.detachedFromTarget.

Even playwright emits the exact same events since these are CDP events, however this connection class doesn’t exist in case of playwright (ref).

Meaning, to make this work with lighthouse, we would need to create a custom adapter connection class that acts as a bridge between playwright and lighthouse (ref).

Will take a look into the feasibility, can discuss if you've any approaches in mind.

Xotabu4 commented 1 year ago

@asambstack i took the code from this PR

const patchPageObject = (page) => {
  page.target = function () {
    return {
      createCDPSession: async function () {
        const session = await page.context().newCDPSession(page);
        session.connection = () => new events.EventEmitter();
        return session;
      },
    };
  };
};

Tried on my own project - and so far it works fine for me, even with page navigations initiated by user and webrtc trafic on the pages.

Will keep you updated in case will see some issues

abhinaba-ghosh commented 9 months ago

@asambstack apologies for a delayed response. Are we still considering moving forward with this PR?

Xotabu4 commented 7 months ago

@asambstack @abhinaba-ghosh any updates on this? Would be cool to have this merged