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

CSS Gen: Don't open all pages at once in Playwright interface #41634

Draft
wants to merge 2 commits into
base: trunk
Choose a base branch
from
+181 −89
Draft

CSS Gen: Don't open all pages at once in Playwright interface #41634

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fce223
Update playwright interface to not open all pages at the same time
dilirity Feb 7, 2025
dab567b
add changelog
dilirity Feb 7, 2025
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
4 changes: 4 additions & 0 deletions ...ages/critical-css-gen/changelog/update-css-gen-lib-playwright-dont-open-all-pages-at-once
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: changed

Update Playwright interface, to open less pages at the same time.
107 changes: 54 additions & 53 deletions projects/js-packages/critical-css-gen/src/browser-interface-playwright.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,62 +16,14 @@ export class BrowserInterfacePlaywright extends BrowserInterface {
* Creates a new BrowserInterfacePlaywright instance.
*
* @param {BrowserContext} context - The playwright browser context to work with.
* @param {string[]} urls - Array of urls to evaluate. The reason we are taking this as an argument is because we want to load all of them in parallel.
* @param {string[]} urls - Array of urls to evaluate.
*/
constructor(
private context: BrowserContext,
private urls: string[]
) {
super();
}

private async getTabs() {
if ( typeof this.tabs === 'undefined' ) {
await this.openUrls( this.context, this.urls );
}

return this.tabs;
}

/**
* Open an array of urls in a new browser context.
*
* Take a browser instance and an array of urls to open in new tabs.
*
* @param {BrowserContext} context - Browser context to use.
* @param {string[]} urls - Array of urls to open.
* @return {Promise< TabsByUrl >} Promise resolving to the browser context.
*/
private async openUrls( context: BrowserContext, urls: string[] ): Promise< void > {
this.tabs = await objectPromiseAll< Tab >(
urls.reduce( ( set, url ) => {
set[ url ] = this.newTab( context, url );
return set;
}, {} )
);
}

/**
* Open url in a new tab in a given browserContext.
*
* @param {BrowserContext} browserContext - Browser context to use.
* @param {string} url - Url to open.
* @return {Promise<Page>} Promise resolving to the page instance.
*/
private async newTab( browserContext: BrowserContext, url: string ): Promise< Tab > {
const tab = {
page: await browserContext.newPage(),
statusCode: null,
};
tab.page.on( 'response', async response => {
if ( response.url() === url ) {
tab.statusCode = response.status();
}
} );

await tab.page.goto( url, { timeout: PAGE_GOTO_TIMEOUT_MS } );

return tab;
this.tabs = {};
}

async runInPage< ReturnType >(
Expand All @@ -80,11 +32,13 @@ export class BrowserInterfacePlaywright extends BrowserInterface {
method: BrowserRunnable< ReturnType >,
...args: unknown[]
): Promise< ReturnType > {
const tabs = await this.getTabs();
const tab = tabs[ pageUrl ];
const tab = this.tabs[ pageUrl ];

if ( ! tab || ! tab.page ) {
throw new Error( `Playwright interface does not include URL ${ pageUrl }` );
if ( ! this.urls.includes( pageUrl ) ) {
throw new Error( `URL not in original URL set: ${ pageUrl }` );
}
throw new Error( `Page not loaded in current batch: ${ pageUrl }` );
}

// Bail early if the page returned a non-200 status code.
Expand Down Expand Up @@ -120,4 +74,51 @@ export class BrowserInterfacePlaywright extends BrowserInterface {
private isOkStatus( statusCode: number ) {
return statusCode >= 200 && statusCode < 300;
}

async loadBatch( urls: string[] ): Promise< void > {
// Close existing tabs
await this.closeTabs();

// Load new batch of URLs
this.tabs = await objectPromiseAll< Tab >(
urls.reduce( ( set, url ) => {
set[ url ] = this.newTab( this.context, url );
return set;
}, {} )
);
}

/**
* Open url in a new tab in a given browserContext.
*
* @param {BrowserContext} browserContext - Browser context to use.
* @param {string} url - Url to open.
* @return {Promise<Tab>} Promise resolving to the tab instance.
*/
private async newTab( browserContext: BrowserContext, url: string ): Promise< Tab > {
const tab = {
page: await browserContext.newPage(),
statusCode: null,
};
tab.page.on( 'response', async response => {
if ( response.url() === url ) {
tab.statusCode = response.status();
}
} );

await tab.page.goto( url, { timeout: PAGE_GOTO_TIMEOUT_MS } );

return tab;
}

private async closeTabs(): Promise< void > {
for ( const tab of Object.values( this.tabs ) ) {
await tab.page.close();
}
this.tabs = {};
}

async cleanup() {
await this.closeTabs();
}
}
Loading
Loading