From 12cff08cc640bcf0c2a7ea253673b340f0ed4d4d Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 28 Aug 2024 14:55:50 -0700 Subject: [PATCH] Revert "fix(addInitScript): require non-undefined arg to trigger commonjs module (#32282)" This reverts commit 3a75f23ea1d116a57f6a9d6bf758cf8d693eb7c3. --- docs/src/api/class-browsercontext.md | 4 ++-- docs/src/api/class-page.md | 4 ++-- packages/playwright-core/src/client/browserContext.ts | 2 +- packages/playwright-core/src/client/clientHelper.ts | 4 ++-- packages/playwright-core/src/client/page.ts | 2 +- packages/playwright-core/src/client/selectors.ts | 2 +- packages/playwright-core/types/types.d.ts | 8 ++++---- tests/page/page-add-init-script.spec.ts | 6 ++++++ 8 files changed, 19 insertions(+), 13 deletions(-) diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index 36f980e0ce..17913b5402 100644 --- a/docs/src/api/class-browsercontext.md +++ b/docs/src/api/class-browsercontext.md @@ -439,9 +439,9 @@ const mockPath = { path: path.resolve(__dirname, '../mocks/mockRandom.js') }; // Passing 42 as an argument to the default export function. await context.addInitScript({ path: mockPath }, 42); -// Make sure to pass something even if you do not need to pass an argument. +// Make sure to pass undefined even if you do not need to pass an argument. // This instructs Playwright to treat the file as a commonjs module. -await context.addInitScript({ path: mockPath }, ''); +await context.addInitScript({ path: mockPath }, undefined); ``` ### param: BrowserContext.addInitScript.script diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index da5d48f906..2eb513e1e3 100644 --- a/docs/src/api/class-page.md +++ b/docs/src/api/class-page.md @@ -643,9 +643,9 @@ const mockPath = { path: path.resolve(__dirname, '../mocks/mockRandom.js') }; // Passing 42 as an argument to the default export function. await page.addInitScript({ path: mockPath }, 42); -// Make sure to pass something even if you do not need to pass an argument. +// Make sure to pass undefined even if you do not need to pass an argument. // This instructs Playwright to treat the file as a commonjs module. -await page.addInitScript({ path: mockPath }, ''); +await page.addInitScript({ path: mockPath }, undefined); ``` ### param: Page.addInitScript.script diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index ef222136dd..9df8f2afad 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -308,7 +308,7 @@ export class BrowserContext extends ChannelOwner } async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any): Promise { - const source = await evaluationScript(script, arg); + const source = await evaluationScript(script, arg, arguments.length > 1); await this._channel.addInitScript({ source }); } diff --git a/packages/playwright-core/src/client/clientHelper.ts b/packages/playwright-core/src/client/clientHelper.ts index 793219f10b..fcc785b71b 100644 --- a/packages/playwright-core/src/client/clientHelper.ts +++ b/packages/playwright-core/src/client/clientHelper.ts @@ -28,7 +28,7 @@ export function envObjectToArray(env: types.Env): { name: string, value: string return result; } -export async function evaluationScript(fun: Function | string | { path?: string, content?: string }, arg: any, addSourceUrl: boolean = true): Promise { +export async function evaluationScript(fun: Function | string | { path?: string, content?: string }, arg: any, hasArg: boolean, addSourceUrl: boolean = true): Promise { if (typeof fun === 'function') { const source = fun.toString(); const argString = Object.is(arg, undefined) ? 'undefined' : JSON.stringify(arg); @@ -46,7 +46,7 @@ export async function evaluationScript(fun: Function | string | { path?: string, } if (fun.path !== undefined) { let source = await fs.promises.readFile(fun.path, 'utf8'); - if (arg !== undefined) { + if (hasArg) { // Assume a CJS module that has a function default export. source = `(() => { var exports = {}; var module = { exports }; diff --git a/packages/playwright-core/src/client/page.ts b/packages/playwright-core/src/client/page.ts index a10286fa9a..5ff6d6178d 100644 --- a/packages/playwright-core/src/client/page.ts +++ b/packages/playwright-core/src/client/page.ts @@ -492,7 +492,7 @@ export class Page extends ChannelOwner implements api.Page } async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any) { - const source = await evaluationScript(script, arg); + const source = await evaluationScript(script, arg, arguments.length > 1); await this._channel.addInitScript({ source }); } diff --git a/packages/playwright-core/src/client/selectors.ts b/packages/playwright-core/src/client/selectors.ts index 2739be0e8d..c7a7967559 100644 --- a/packages/playwright-core/src/client/selectors.ts +++ b/packages/playwright-core/src/client/selectors.ts @@ -26,7 +26,7 @@ export class Selectors implements api.Selectors { private _registrations: channels.SelectorsRegisterParams[] = []; async register(name: string, script: string | (() => SelectorEngine) | { path?: string, content?: string }, options: { contentScript?: boolean } = {}): Promise { - const source = await evaluationScript(script, undefined, false); + const source = await evaluationScript(script, undefined, false, false); const params = { ...options, name, source }; for (const channel of this._channels) await channel._channel.register(params); diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index d0424c3024..eabcf45f3f 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -312,9 +312,9 @@ export interface Page { * // Passing 42 as an argument to the default export function. * await page.addInitScript({ path: mockPath }, 42); * - * // Make sure to pass something even if you do not need to pass an argument. + * // Make sure to pass undefined even if you do not need to pass an argument. * // This instructs Playwright to treat the file as a commonjs module. - * await page.addInitScript({ path: mockPath }, ''); + * await page.addInitScript({ path: mockPath }, undefined); * ``` * * @param script Script to be evaluated in the page. @@ -7757,9 +7757,9 @@ export interface BrowserContext { * // Passing 42 as an argument to the default export function. * await context.addInitScript({ path: mockPath }, 42); * - * // Make sure to pass something even if you do not need to pass an argument. + * // Make sure to pass undefined even if you do not need to pass an argument. * // This instructs Playwright to treat the file as a commonjs module. - * await context.addInitScript({ path: mockPath }, ''); + * await context.addInitScript({ path: mockPath }, undefined); * ``` * * @param script Script to be evaluated in all pages in the browser context. diff --git a/tests/page/page-add-init-script.spec.ts b/tests/page/page-add-init-script.spec.ts index 2c8234a550..2b12f00251 100644 --- a/tests/page/page-add-init-script.spec.ts +++ b/tests/page/page-add-init-script.spec.ts @@ -37,6 +37,12 @@ it('should assume CJS module with a path and arg', async ({ page, server, asset expect(await page.evaluate(() => window['injected'])).toBe(17); }); +it('should assume CJS module with a path and undefined arg', async ({ page, server, asset }) => { + await page.addInitScript({ path: asset('injectedmodule.js') }, undefined); + await page.goto(server.EMPTY_PAGE); + expect(await page.evaluate(() => window['injected'])).toBe(42); +}); + it('should work with content @smoke', async ({ page, server }) => { await page.addInitScript({ content: 'window["injected"] = 123' }); await page.goto(server.PREFIX + '/tamperable.html');