From 5ce3abe675ab9db9c04751805df5e837d3b1094d Mon Sep 17 00:00:00 2001 From: Nikolay Vitkov <34244704+Lightning00Blade@users.noreply.github.com> Date: Mon, 16 Oct 2023 14:37:52 +0200 Subject: [PATCH] chore: use RxJS instead of waitWithTimeout (#11160) --- packages/puppeteer-core/src/bidi/Frame.ts | 22 +++++------- packages/puppeteer-core/src/bidi/Page.ts | 36 ++++++++++--------- packages/puppeteer-core/src/cdp/Page.ts | 11 +++--- .../puppeteer-core/third_party/rxjs/rxjs.ts | 1 + test/src/locator.spec.ts | 20 +++++------ 5 files changed, 44 insertions(+), 46 deletions(-) diff --git a/packages/puppeteer-core/src/bidi/Frame.ts b/packages/puppeteer-core/src/bidi/Frame.ts index c1bf67cb..64085d64 100644 --- a/packages/puppeteer-core/src/bidi/Frame.ts +++ b/packages/puppeteer-core/src/bidi/Frame.ts @@ -24,6 +24,8 @@ import { merge, raceWith, switchMap, + forkJoin, + first, } from '../../third_party/rxjs/rxjs.js'; import type {CDPSession} from '../api/CDPSession.js'; import { @@ -179,25 +181,19 @@ export class BidiFrame extends Frame { ): Promise { const { waitUntil = 'load', - timeout = this.#timeoutSettings.navigationTimeout(), + timeout: ms = this.#timeoutSettings.navigationTimeout(), } = options; const waitUntilEvent = lifeCycleToSubscribedEvent.get( getWaitUntilSingle(waitUntil) ) as string; - await Promise.all([ - setPageContent(this, html), - waitWithTimeout( - new Promise(resolve => { - this.#context.once(waitUntilEvent, () => { - resolve(); - }); - }), - waitUntilEvent, - timeout - ), - ]); + await firstValueFrom( + forkJoin([ + fromEvent(this.#context, waitUntilEvent).pipe(first()), + from(setPageContent(this, html)), + ]).pipe(raceWith(timeout(ms))) + ); } context(): BrowsingContext { diff --git a/packages/puppeteer-core/src/bidi/Page.ts b/packages/puppeteer-core/src/bidi/Page.ts index 0f0f34f4..dc5e298a 100644 --- a/packages/puppeteer-core/src/bidi/Page.ts +++ b/packages/puppeteer-core/src/bidi/Page.ts @@ -18,6 +18,7 @@ import type {Readable} from 'stream'; import type * as Bidi from 'chromium-bidi/lib/cjs/protocol/protocol.js'; import type Protocol from 'devtools-protocol'; +import {firstValueFrom, from, raceWith} from 'rxjs'; import type {CDPSession} from '../api/CDPSession.js'; import type {WaitForOptions} from '../api/Frame.js'; @@ -50,6 +51,7 @@ import type {Awaitable} from '../common/types.js'; import { debugError, evaluationString, + timeout, validateDialogType, waitForHTTP, waitWithTimeout, @@ -599,25 +601,25 @@ export class BidiPage extends Page { pageRanges: ranges, scale, preferCSSPageSize, - timeout, + timeout: ms, } = this._getPDFOptions(options, 'cm'); const pageRanges = ranges ? ranges.split(', ') : []; - const {result} = await waitWithTimeout( - this.#connection.send('browsingContext.print', { - context: this.mainFrame()._id, - background, - margin, - orientation: landscape ? 'landscape' : 'portrait', - page: { - width, - height, - }, - pageRanges, - scale, - shrinkToFit: !preferCSSPageSize, - }), - 'browsingContext.print', - timeout + const {result} = await firstValueFrom( + from( + this.#connection.send('browsingContext.print', { + context: this.mainFrame()._id, + background, + margin, + orientation: landscape ? 'landscape' : 'portrait', + page: { + width, + height, + }, + pageRanges, + scale, + shrinkToFit: !preferCSSPageSize, + }) + ).pipe(raceWith(timeout(ms))) ); const buffer = Buffer.from(result.data, 'base64'); diff --git a/packages/puppeteer-core/src/cdp/Page.ts b/packages/puppeteer-core/src/cdp/Page.ts index 260b6c00..722ac4ea 100644 --- a/packages/puppeteer-core/src/cdp/Page.ts +++ b/packages/puppeteer-core/src/cdp/Page.ts @@ -18,6 +18,7 @@ import type {Readable} from 'stream'; import type {Protocol} from 'devtools-protocol'; +import {firstValueFrom, from, raceWith} from '../../third_party/rxjs/rxjs.js'; import type {Browser} from '../api/Browser.js'; import type {BrowserContext} from '../api/BrowserContext.js'; import {CDPSessionEvent, type CDPSession} from '../api/CDPSession.js'; @@ -53,10 +54,10 @@ import { getReadableAsBuffer, getReadableFromProtocolStream, pageBindingInitString, + timeout, validateDialogType, valueFromRemoteObject, waitForHTTP, - waitWithTimeout, } from '../common/util.js'; import type {Viewport} from '../common/Viewport.js'; import {assert} from '../util/assert.js'; @@ -1138,7 +1139,7 @@ export class CdpPage extends Page { pageRanges, preferCSSPageSize, omitBackground, - timeout, + timeout: ms, } = this._getPDFOptions(options); if (omitBackground) { @@ -1166,10 +1167,8 @@ export class CdpPage extends Page { } ); - const result = await waitWithTimeout( - printCommandPromise, - 'Page.printToPDF', - timeout + const result = await firstValueFrom( + from(printCommandPromise).pipe(raceWith(timeout(ms))) ); if (omitBackground) { diff --git a/packages/puppeteer-core/third_party/rxjs/rxjs.ts b/packages/puppeteer-core/third_party/rxjs/rxjs.ts index 4ab42672..503f1044 100644 --- a/packages/puppeteer-core/third_party/rxjs/rxjs.ts +++ b/packages/puppeteer-core/third_party/rxjs/rxjs.ts @@ -47,6 +47,7 @@ export { tap, throwIfEmpty, timer, + forkJoin, } from 'rxjs'; import {filter, from, map, mergeMap, type Observable} from 'rxjs'; diff --git a/test/src/locator.spec.ts b/test/src/locator.spec.ts index 26b2e352..35023995 100644 --- a/test/src/locator.spec.ts +++ b/test/src/locator.spec.ts @@ -18,7 +18,7 @@ import expect from 'expect'; import {TimeoutError} from 'puppeteer-core'; import { Locator, - LocatorEmittedEvents, + LocatorEvent, } from 'puppeteer-core/internal/api/locators/locators.js'; import sinon from 'sinon'; @@ -38,7 +38,7 @@ describe('Locator', function () { await page .mainFrame() .locator('button') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { willClick = true; }) .click(); @@ -65,7 +65,7 @@ describe('Locator', function () { .setVisibility(null) .setWaitForEnabled(false) .setWaitForStableBoundingBox(false) - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { willClick = true; }) .click(); @@ -88,7 +88,7 @@ describe('Locator', function () { let willClick = false; await page .locator('button') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { willClick = true; }) .click(); @@ -110,7 +110,7 @@ describe('Locator', function () { let clicked = false; await page .locator('::-p-text(test), ::-p-xpath(/button)') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { clicked = true; }) .click(); @@ -304,7 +304,7 @@ describe('Locator', function () { let willClick = false; await frame .locator('button') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { willClick = true; }) .click(); @@ -328,7 +328,7 @@ describe('Locator', function () { let hovered = false; await page .locator('button') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { hovered = true; }) .hover(); @@ -354,7 +354,7 @@ describe('Locator', function () { let scrolled = false; await page .locator('div') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { scrolled = true; }) .scroll({ @@ -380,7 +380,7 @@ describe('Locator', function () { let filled = false; await page .locator('textarea') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { filled = true; }) .fill('test'); @@ -404,7 +404,7 @@ describe('Locator', function () { let filled = false; await page .locator('select') - .on(LocatorEmittedEvents.Action, () => { + .on(LocatorEvent.Action, () => { filled = true; }) .fill('value2');