Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: types in Browser.ts to be compatible with strict mode Typescript #7918

Merged
merged 1 commit into from Jan 20, 2022
Merged

fix: types in Browser.ts to be compatible with strict mode Typescript #7918

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 41 additions & 11 deletions src/common/Browser.ts
View file
Open in desktop
Expand Up @@ -268,7 +268,7 @@ export class Browser extends EventEmitter {
this._closeCallback = closeCallback || function (): void {};
this._targetFilterCallback = targetFilterCallback || ((): boolean => true);

this._defaultContext = new BrowserContext(this._connection, this, null);
this._defaultContext = new BrowserContext(this._connection, this);
this._contexts = new Map();
for (const contextId of contextIds)
this._contexts.set(
Expand Down Expand Up @@ -296,7 +296,7 @@ export class Browser extends EventEmitter {
* {@link Puppeteer.connect}.
*/
process(): ChildProcess | null {
return this._process;
return this._process ?? null;
}

/**
Expand Down Expand Up @@ -357,8 +357,11 @@ export class Browser extends EventEmitter {
* Used by BrowserContext directly so cannot be marked private.
*/
async _disposeContext(contextId?: string): Promise<void> {
if (!contextId) {
return;
}
await this._connection.send('Target.disposeBrowserContext', {
browserContextId: contextId || undefined,
browserContextId: contextId,
});
this._contexts.delete(contextId);
}
Expand All @@ -373,6 +376,10 @@ export class Browser extends EventEmitter {
? this._contexts.get(browserContextId)
: this._defaultContext;

if (!context) {
throw new Error('Missing browser context');
}

const shouldAttachToTarget = this._targetFilterCallback(targetInfo);
if (!shouldAttachToTarget) {
this._ignoredTargets.add(targetInfo.targetId);
Expand All @@ -384,7 +391,7 @@ export class Browser extends EventEmitter {
context,
() => this._connection.createSession(targetInfo),
this._ignoreHTTPSErrors,
this._defaultViewport,
this._defaultViewport ?? null,
this._screenshotTaskQueue
);
assert(
Expand All @@ -402,6 +409,11 @@ export class Browser extends EventEmitter {
private async _targetDestroyed(event: { targetId: string }): Promise<void> {
if (this._ignoredTargets.has(event.targetId)) return;
const target = this._targets.get(event.targetId);
if (!target) {
throw new Error(
`Missing target in _targetDestroyed (id = ${event.targetId})`
);
}
target._initializedCallback(false);
this._targets.delete(event.targetId);
target._closedCallback();
Expand All @@ -418,7 +430,11 @@ export class Browser extends EventEmitter {
): void {
if (this._ignoredTargets.has(event.targetInfo.targetId)) return;
const target = this._targets.get(event.targetInfo.targetId);
assert(target, 'target should exist before targetInfoChanged');
if (!target) {
throw new Error(
`Missing target in targetInfoChanged (id = ${event.targetInfo.targetId})`
);
}
const previousURL = target.url();
const wasInitialized = target._isInitialized;
target._targetInfoChanged(event.targetInfo);
Expand Down Expand Up @@ -469,11 +485,19 @@ export class Browser extends EventEmitter {
browserContextId: contextId || undefined,
});
const target = this._targets.get(targetId);
assert(
await target._initializedPromise,
'Failed to create target for page'
);
if (!target) {
throw new Error(`Missing target for page (id = ${targetId})`);
}
const initialized = await target._initializedPromise;
if (!initialized) {
throw new Error(`Failed to create target for page (id = ${targetId})`);
}
const page = await target.page();
if (!page) {
throw new Error(
`Failed to create a page for context (id = ${contextId})`
);
}
return page;
}

Expand All @@ -491,7 +515,13 @@ export class Browser extends EventEmitter {
* The target associated with the browser.
*/
target(): Target {
return this.targets().find((target) => target.type() === 'browser');
const browserTarget = this.targets().find(
(target) => target.type() === 'browser'
);
if (!browserTarget) {
throw new Error('Browser target is not found');
}
return browserTarget;
}

/**
Expand Down Expand Up @@ -728,7 +758,7 @@ export class BrowserContext extends EventEmitter {
.filter((target) => target.type() === 'page')
.map((target) => target.page())
);
return pages.filter((page) => !!page);
return pages.filter((page): page is Page => !!page);
}

/**
Expand Down