Skip to content

Commit

Permalink
fix(addInitScript): require non-undefined arg to trigger commonjs mod…
Browse files Browse the repository at this point in the history
  • Loading branch information
dgozman authored Aug 23, 2024
1 parent 0b9c036 commit 3a75f23
Show file tree
Hide file tree
Showing 8 changed files with 13 additions and 19 deletions.
4 changes: 2 additions & 2 deletions docs/src/api/class-browsercontext.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions docs/src/api/class-page.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
}

async addInitScript(script: Function | string | { path?: string, content?: string }, arg?: any): Promise<void> {
const source = await evaluationScript(script, arg, arguments.length > 1);
const source = await evaluationScript(script, arg);
await this._channel.addInitScript({ source });
}

Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/client/clientHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> {
export async function evaluationScript(fun: Function | string | { path?: string, content?: string }, arg: any, addSourceUrl: boolean = true): Promise<string> {
if (typeof fun === 'function') {
const source = fun.toString();
const argString = Object.is(arg, undefined) ? 'undefined' : JSON.stringify(arg);
Expand All @@ -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 };
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/client/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ export class Page extends ChannelOwner<channels.PageChannel> 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 });
}

Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/client/selectors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
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);
Expand Down
8 changes: 4 additions & 4 deletions packages/playwright-core/types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
6 changes: 0 additions & 6 deletions tests/page/page-add-init-script.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit 3a75f23

Please sign in to comment.