feat(browser): browser.on(context)#40315
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| * since: v1.60 | ||
| - argument: <[BrowserContext]> | ||
|
|
||
| Emitted when a new browser context is created. This event is also emitted for contexts created by other clients |
There was a problem hiding this comment.
Don't mention or advertise the Browser.bind.
| type BrowserSlot = { | ||
| guid: string; | ||
| contextGuid: string; | ||
| contextGuid?: string; |
There was a problem hiding this comment.
These are never good, let's flatten browser/context early to avoid nullability.
| if (!slot) | ||
| if (!slot?.context) | ||
| return; | ||
| const attached = new AttachedBrowser(this, slot); |
There was a problem hiding this comment.
this will be a browser/context pair
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
let's do #40333 first |
Skn0tt
force-pushed
the
feat-browser-on-context
branch
2 times, most recently
from
201b051 to
b56a119
Compare
Skn0tt
force-pushed
the
feat-browser-on-context
branch
from
b56a119 to
1d5286c
Compare
dgozman
left a comment
dgozman
left a comment
There was a problem hiding this comment.
I'd expect test coverage for the new public API!
| private _didCreateContext(context: BrowserContext) { | ||
| context._browser = this; | ||
| this._contexts.add(context); | ||
| this.emit(Events.Browser.Context, context); |
There was a problem hiding this comment.
Please move this to the last line of the method, so that context is fully initialized already.
| const userDataDirs = await fs.promises.readdir(daemonDir).catch(() => []); | ||
| for (const dir of userDataDirs.filter(f => f.startsWith('ud-'))) | ||
| await fs.promises.rm(path.join(daemonDir, dir), { recursive: true, force: true }).catch(() => {}); | ||
| const daemonDir = test.info().outputPath('daemon'); |
There was a problem hiding this comment.
This change seems entirely unrelated!
There was a problem hiding this comment.
absolutely, but it was useful utility for having copilot find the bug this fixes by itself.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Test results for "tests 1"9 flaky41317 passed, 847 skipped Merge workflow run. |
Test results for "MCP"3 failed 1 flaky6561 passed, 910 skipped Merge workflow run. |
3 participants
Summary
browser.on('context', ...)event, fired when a new context becomes visible viabrowser.contexts().