diff --git a/src/PageCollector.ts b/src/PageCollector.ts index cb7618b..9b078d5 100644 --- a/src/PageCollector.ts +++ b/src/PageCollector.ts @@ -9,6 +9,11 @@ import type {Browser, HTTPRequest, Page} from 'puppeteer-core'; export class PageCollector { #browser: Browser; #initializer: (page: Page, collector: (item: T) => void) => void; + /** + * The Array in this map should only be set once + * As we use the reference to it. + * Use methods that manipulate the array in place. + */ protected storage = new WeakMap(); constructor( @@ -30,7 +35,6 @@ export class PageCollector { if (!page) { return; } - this.#initializePage(page); }); } @@ -44,6 +48,9 @@ export class PageCollector { return; } + const stored: T[] = []; + this.storage.set(page, stored); + page.on('framenavigated', frame => { // Only reset the storage on main frame navigation if (frame !== page.mainFrame()) { @@ -52,16 +59,16 @@ export class PageCollector { this.cleanup(page); }); this.#initializer(page, value => { - const stored = this.storage.get(page) ?? []; stored.push(value); - this.storage.set(page, stored); }); } protected cleanup(page: Page) { - const collection = this.storage.get(page) ?? []; - // Keep the reference alive - collection.length = 0; + const collection = this.storage.get(page); + if (collection) { + // Keep the reference alive + collection.length = 0; + } } getData(page: Page): T[] { @@ -72,6 +79,9 @@ export class PageCollector { export class NetworkCollector extends PageCollector { override cleanup(page: Page) { const requests = this.storage.get(page) ?? []; + if (!requests) { + return; + } const lastRequestIdx = requests.findLastIndex(request => { return request.frame() === page.mainFrame() ? request.isNavigationRequest() @@ -79,6 +89,7 @@ export class NetworkCollector extends PageCollector { }); // Keep all requests since the last navigation request including that // navigation request itself. - this.storage.set(page, requests.slice(Math.max(lastRequestIdx, 0))); + // Keep the reference + requests.splice(0, Math.max(lastRequestIdx, 0)); } } diff --git a/tests/PageCollector.test.ts b/tests/PageCollector.test.ts index ce61ac8..0e8248c 100644 --- a/tests/PageCollector.test.ts +++ b/tests/PageCollector.test.ts @@ -6,25 +6,37 @@ import assert from 'node:assert'; import {describe, it} from 'node:test'; -import type {Browser, Frame, Page, PageEvents} from 'puppeteer-core'; +import type {Browser, Frame, Page, Target} from 'puppeteer-core'; import {PageCollector} from '../src/PageCollector.js'; import {getMockRequest} from './utils.js'; -function getMockPage(): Page { - const listeners: Record void> = {}; - const mainFrame = {} as Frame; +function mockListener() { + const listeners: Record void>> = {}; return { - on(eventName, listener) { - listeners[eventName] = listener; + on(eventName: string, listener: (data: unknown) => void) { + if (listeners[eventName]) { + listeners[eventName].push(listener); + } else { + listeners[eventName] = [listener]; + } }, - emit(eventName, data) { - listeners[eventName]?.(data); + emit(eventName: string, data: unknown) { + for (const listener of listeners[eventName] ?? []) { + listener(data); + } }, + }; +} + +function getMockPage(): Page { + const mainFrame = {} as Frame; + return { mainFrame() { return mainFrame; }, + ...mockListener(), } as Page; } @@ -34,9 +46,7 @@ function getMockBrowser(): Browser { pages() { return Promise.resolve(pages); }, - on(_type, _handler) { - // Mock - }, + ...mockListener(), } as Browser; } @@ -113,4 +123,34 @@ describe('PageCollector', () => { assert.equal(collector.getData(page).length, 1); }); + + it('should only subscribe once', async () => { + const browser = getMockBrowser(); + const page = (await browser.pages())[0]; + const request = getMockRequest(); + const collector = new PageCollector(browser, (pageListener, collect) => { + pageListener.on('request', req => { + collect(req); + }); + }); + await collector.init(); + browser.emit('targetcreated', { + page() { + return Promise.resolve(page); + }, + } as Target); + + // The page inside part is async so we need to await some time + await new Promise(res => res()); + + assert.equal(collector.getData(page).length, 0); + + page.emit('request', request); + + assert.equal(collector.getData(page).length, 1); + + page.emit('request', request); + + assert.equal(collector.getData(page).length, 2); + }); });