Skip to content

Commit de87ef2

Browse files
authored
chore: don't let count() throw during navigation (#36941)
1 parent 4672d98 commit de87ef2

File tree

7 files changed

+24
-9
lines changed

7 files changed

+24
-9
lines changed

packages/playwright-core/src/client/frame.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,8 @@ export class Frame extends ChannelOwner<channels.FrameChannel> implements api.Fr
247247
return result.elements.map(e => ElementHandle.from(e) as ElementHandle<SVGElement | HTMLElement>);
248248
}
249249

250-
async _queryCount(selector: string): Promise<number> {
251-
return (await this._channel.queryCount({ selector })).value;
250+
async _queryCount(selector: string, options?: {}): Promise<number> {
251+
return (await this._channel.queryCount({ selector, ...options })).value;
252252
}
253253

254254
async content(): Promise<string> {

packages/playwright-core/src/client/locator.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,8 +249,9 @@ export class Locator implements api.Locator {
249249
await this._frame._channel.blur({ selector: this._selector, strict: true, ...options, timeout: this._frame._timeout(options) });
250250
}
251251

252-
async count(): Promise<number> {
253-
return await this._frame._queryCount(this._selector);
252+
// options are only here for testing
253+
async count(_options?: {}): Promise<number> {
254+
return await this._frame._queryCount(this._selector, _options);
254255
}
255256

256257
async _resolveSelector(): Promise<{ resolvedSelector: string }> {

packages/playwright-core/src/server/dispatchers/frameDispatcher.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ export class FrameDispatcher extends Dispatcher<Frame, channels.FrameChannel, Br
117117
}
118118

119119
async queryCount(params: channels.FrameQueryCountParams, progress: Progress): Promise<channels.FrameQueryCountResult> {
120-
return { value: await progress.race(this._frame.queryCount(params.selector)) };
120+
return { value: await progress.race(this._frame.queryCount(params.selector, params)) };
121121
}
122122

123123
async content(params: channels.FrameContentParams, progress: Progress): Promise<channels.FrameContentResult> {

packages/playwright-core/src/server/frameSelectors.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,12 @@ export class FrameSelectors {
7575
}, { info: resolved.info, scope: resolved.scope });
7676
}
7777

78-
async queryCount(selector: string): Promise<number> {
78+
async queryCount(selector: string, options: any): Promise<number> {
7979
const resolved = await this.resolveInjectedForSelector(selector);
8080
// Be careful, |this.frame| can be different from |resolved.frame|.
8181
if (!resolved)
8282
throw new Error(`Failed to find frame for selector "${selector}"`);
83+
await options.__testHookBeforeQuery?.();
8384
return await resolved.injected.evaluate((injected, { info }) => {
8485
return injected.querySelectorAll(info.parsed, document).length;
8586
}, { info: resolved.info });

packages/playwright-core/src/server/frames.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -841,8 +841,14 @@ export class Frame extends SdkObject {
841841
return this.selectors.queryAll(selector);
842842
}
843843

844-
async queryCount(selector: string): Promise<number> {
845-
return await this.selectors.queryCount(selector);
844+
async queryCount(selector: string, options: any): Promise<number> {
845+
try {
846+
return await this.selectors.queryCount(selector, options);
847+
} catch (e) {
848+
if (this.isNonRetriableError(e))
849+
throw e;
850+
return 0;
851+
}
846852
}
847853

848854
async content(): Promise<string> {

tests/page/locator-query.spec.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,3 +272,10 @@ it('alias methods coverage', async ({ page }) => {
272272
await expect(page.locator('div').getByRole('button')).toHaveCount(1);
273273
await expect(page.mainFrame().locator('button')).toHaveCount(1);
274274
});
275+
276+
it('count() should not throw during navigation', async ({ page }) => {
277+
await page.setContent(`<div>A</div>`);
278+
const __testHookBeforeQuery = () => page.goto('data:text/html,<div>A</div><div>B</div>');
279+
// @ts-expect-error
280+
expect(await page.locator('div').count({ __testHookBeforeQuery })).toBe(0);
281+
});

utils/doclint/missingDocs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ module.exports = function lint(documentation, jsSources, apiFileName) {
4242
}
4343
for (const paramName of params) {
4444
const found = members.some(member => paramsForMember(member).has(paramName));
45-
if (!found)
45+
if (!found && !paramName.startsWith('_'))
4646
errors.push(`Missing documentation for "${className}.${methodName}.${paramName}"`);
4747
}
4848
}

0 commit comments

Comments
 (0)