From 3a75f23ea1d116a57f6a9d6bf758cf8d693eb7c3 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 23 Aug 2024 02:48:56 -0700 Subject: [PATCH] fix(addInitScript): require non-undefined arg to trigger commonjs module (#32282) --- 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, 13 insertions(+), 19 deletions(-) diff --git a/docs/src/api/class-browsercontext.md b/docs/src/api/class-browsercontext.md index 7b6c6aab1d..eed27fc1c1 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 undefined even if you do not need to pass an argument. +// Make sure to pass something 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 }, undefined); +await context.addInitScript({ path: mockPath }, ''); ``` ### param: BrowserContext.addInitScript.script diff --git a/docs/src/api/class-page.md b/docs/src/api/class-page.md index 11d7cbdd74..78d38a6f7a 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 undefined even if you do not need to pass an argument. +// Make sure to pass something 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 }, undefined); +await page.addInitScript({ path: mockPath }, ''); ``` ### param: Page.addInitScript.script diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index b0b72917cf..c4f7827840 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, arguments.length > 1); + const source = await evaluationScript(script, arg); await this._channel.addInitScript({ source }); } diff --git a/packages/playwright-core/src/client/clientHelper.ts b/packages/playwright-core/src/client/clientHelper.ts index fcc785b71b..793219f10b 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, hasArg: boolean, addSourceUrl: boolean = true): Promise { +export async function evaluationScript(fun: Function | string | { path?: string, content?: string }, arg: any, 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 (hasArg) { + if (arg !== undefined) { // 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 5ff6d6178d..a10286fa9a 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, arguments.length > 1); + const source = await evaluationScript(script, arg); await this._channel.addInitScript({ source }); } diff --git a/packages/playwright-core/src/client/selectors.ts b/packages/playwright-core/src/client/selectors.ts index c7a7967559..2739be0e8d 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, false); + const source = await evaluationScript(script, undefined, 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 0d018dcfd3..f4baa9a352 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 undefined even if you do not need to pass an argument. + * // Make sure to pass something 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 }, undefined); + * await page.addInitScript({ path: mockPath }, ''); * ``` * * @param script Script to be evaluated in the page. @@ -7723,9 +7723,9 @@ export interface BrowserContext { * // Passing 42 as an argument to the default export function. * await context.addInitScript({ path: mockPath }, 42); * - * // Make sure to pass undefined even if you do not need to pass an argument. + * // Make sure to pass something 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 }, undefined); + * await context.addInitScript({ path: mockPath }, ''); * ``` * * @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 2b12f00251..2c8234a550 100644 --- a/tests/page/page-add-init-script.spec.ts +++ b/tests/page/page-add-init-script.spec.ts @@ -37,12 +37,6 @@ 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');