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

fix: extends ElementHandle to Nodes #8552

Merged
merged 2 commits into from Jul 6, 2022
Merged

fix: extends ElementHandle to Nodes #8552

merged 2 commits into from Jul 6, 2022

Conversation

jrandolf
Copy link
Contributor

@jrandolf jrandolf commented Jun 23, 2022

This PR adjusts some incorrect types within Puppeteer:

  • The base type parameter for ElementHandle is now Node instead of Element.
  • The default type parameter for ElementHandle is still Element.
  • QueryHandlers now use Node types.

@jrandolf jrandolf requested a review from OrKoN June 23, 2022 03:21
@jrandolf jrandolf added this to the Puppeteer Type Improvements milestone Jun 23, 2022
@jrandolf jrandolf marked this pull request as ready for review June 23, 2022 03:30
@jrandolf jrandolf force-pushed the node_handle2 branch 5 times, most recently from 234aaf3 to 83ca4ef Compare June 23, 2022 05:06
@OrKoN
Copy link
Collaborator

OrKoN commented Jun 23, 2022

In what cases can an ElementHandle contain a Node?

@jrandolf
Copy link
Contributor Author

In what cases can an ElementHandle contain a Node?

See _createJSHandle. Basically, ElementHandles are created from Nodes (not Elements). For example, evaluateHandle(() => document) will return a valid ElementHandle. Similarly for queries involving ShadowRoots.

@jrandolf jrandolf force-pushed the node_handle2 branch 3 times, most recently from 479d398 to 0c684ee Compare June 23, 2022 07:03
@jrandolf jrandolf changed the base branch from main to node_handle June 23, 2022 07:04
@jrandolf jrandolf force-pushed the node_handle branch 2 times, most recently from 1df828f to d9846f1 Compare June 23, 2022 08:04
@OrKoN
Copy link
Collaborator

OrKoN commented Jun 23, 2022

are any updates needed for api.md for these changes?

@jrandolf
Copy link
Contributor Author

are any updates needed for api.md for these changes?

There are a lot of updates for api.md, but DocLint doesn't support the advanced typings made in previous stages. I propose we merge this and hold on release until we get some documentation (should be complete by next week).

@OrKoN
Copy link
Collaborator

OrKoN commented Jun 23, 2022

@jrandolf wdyt about doing a release now (I assume previous PRs don't need api.md changes) and postponing landing this until the documentation is ready? Today or tomorrow we also need to roll M104 which would be nice to release as soon as possible too so I would not like to block a release on anything.

@jrandolf jrandolf force-pushed the node_handle2 branch 3 times, most recently from fce1d39 to 9099f7b Compare July 5, 2022 16:24
@jrandolf jrandolf changed the title feat!: ElementHandle<Node> return types. feat!: extends ElementHandle to Nodes Jul 5, 2022
@jrandolf jrandolf changed the title feat!: extends ElementHandle to Nodes fix: extends ElementHandle to Nodes Jul 6, 2022
@jrandolf jrandolf merged commit 5ff205d into main Jul 6, 2022
@jrandolf jrandolf deleted the node_handle2 branch July 6, 2022 07:05
Sparticuz added a commit to Sparticuz/chrome-aws-lambda that referenced this pull request Jul 13, 2022
@Qlub53
Copy link

Qlub53 commented Aug 28, 2022

I used to use page.$x(expression) and elementHandle.click([options]) with puppeteer v13.4 just fine.

import puppeteer from 'puppeteer';

const browser = await puppeteer.launch();
const page = await browser.newPage();
await page.goto('https://example.com');
const handles = await page.$x('xpath');
await handles[0].click();

But now since the API signatures have changed ($x(expression) and click(this, options)), the above usage ends up with the following tsc compilation error:

error TS2684: The 'this' context of type 'ElementHandle<Node>' is not assignable to method's 'this' of type 'ElementHandle<Element>'.

7 await handles[0].click();
        ~~~~~~~~~~

How shall I transit from using Element to using Node?

@Qlub53
Copy link

Qlub53 commented Aug 29, 2022

I was able to get away with this problem by type casting.

await (handles[0] as ElementHandle<Element>).click();

Turns out it in fact still works at runtime and only bugs me during compile time.

It's not pretty, but it works. :/

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

Successfully merging this pull request may close these issues.

None yet

3 participants