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.

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

feat: add option to filter targets #7192

Merged
merged 4 commits into from May 3, 2021
Merged

feat: add option to filter targets #7192

merged 4 commits into from May 3, 2021

Conversation

jschfflr
Copy link
Contributor

This change closes #7191 by introducing a callback that can be used to decide if puppeteer should connect to a given target.

src/common/Browser.ts Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
@jschfflr jschfflr merged commit ec3fc2e into main May 3, 2021
@jschfflr jschfflr deleted the target-filter branch May 3, 2021 11:48
@peterbe
Copy link

peterbe commented May 4, 2021

I have no idea how this PR works or what the changes mean, but in the release change log for version 9.1.0 this PR is the only one in that release.
Ever since upgrading to puppeteer 9.1.0 we started seeing strange errors from jest.
Our project has a bunch of *.test.js files and only a couple of them have anything to do with puppeteer. There are a couple that use jest-puppeteer.

Unscientifically, I found that our test suite stopped being so busted when we downgraded back to puppeteer 9.0.0.
Not sure who or how it helps.

Here's my issue about it
Here's my PR to hack around it

@jschfflr
Copy link
Contributor Author

jschfflr commented May 4, 2021

Hi @peterbe Thanks for bringing this to my attention!
I'll try to reproduce the problem and see what this might be about.

@jschfflr
Copy link
Contributor Author

jschfflr commented May 4, 2021

@peterbe Thanks for pinging - I think this is related to the fact that the filter can be async. I'll verify that now...

@peterbe
Copy link

peterbe commented May 4, 2021

What's a "filter" :)

@jschfflr
Copy link
Contributor Author

jschfflr commented May 4, 2021

By default, puppeteer connects to all available targets (=pages and workers etc.). We wanted to give developers a way to control which ones to connect because puppeteer enables some CDP domains by default which might interfere with what that target is doing.
Unfortunately, I allowed the filter to be async and awaited its result. That led to a situation in which the targetInfoChanged event was processed before the target was created because it was still waiting for the return of the targetFilter.

@jribbens
Copy link

jribbens commented May 4, 2021

Yeah I have just raised issue #7204 which turns out to be due to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to ignore certain targets
6 participants