feat(types): add (and fix) evaluateHandle types
#6130
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@hanselfmu heads up that this touches |
jackfranklin
commented
Jun 30, 2020
| // and should use evaluate<ElementHandle> or if the type of evaluateHandle | ||
| // should change to enable the user to tell us they are expecting an | ||
| // ElementHandle rather than the default JSHandle. | ||
| await (buttonHandle as ElementHandle).click(); |
There was a problem hiding this comment.
it's great that this PR enables this big TODO to go!
|
Thanks for doing this 👍 The docs you put together are much better! Let’s be precise when describing this change: |
docs/api.md
Outdated
|
|
||
| The only difference between `worker.evaluate` and `worker.evaluateHandle` is that `worker.evaluateHandle` returns in-page object (JSHandle). | ||
|
|
||
| If the function passed to the `worker.evaluateHandle` returns a [Promise], then `worker.evaluateHandle` would wait for the promise to resolve and return its value. | ||
|
|
||
| If the function returns an element, the returned handle will be of type [ElementHandle]. |
There was a problem hiding this comment.
Stoyan once taught me this trick, sharing in case you like it too: https://twitter.com/mathias/status/839160485324947456
If so, will be → is throughout the doc
There was a problem hiding this comment.
Nice, updated.
jackfranklin
force-pushed
the
evaluate-handle-types
branch
2 times, most recently
from
bde11f7
to
52422aa
Compare
|
I've updated the commit message to clarify that ElementHandle extends JSHandle. So the original docs were technically correct, if not a little misleading / lacking clarity. |
jackfranklin
force-pushed
the
evaluate-handle-types
branch
3 times, most recently
from
e58dcc1
to
3f585b5
Compare
mathiasbynens
approved these changes
Jul 1, 2020
This change started as a small change to pull types from DefinitelyTyped over to
Puppeteer for the `evaluateHandle` function but instead ended up also fixing
what looks to be a long standing issue with our existing documentation.
`evaluateHandle` can in fact return an `ElementHandle` rather than a `JSHandle`.
Note that `ElementHandle` extends `JSHandle` so whilst the docs are technically
correct (all ElementHandles are JSHandles) it's confusing because JSHandles
don't have methods like `click` on them, but ElementHandles do.
if you return something that is an HTML element:
```
const button = page.evaluateHandle(() => document.querySelector('button'));
// this is an ElementHandle, not a JSHandle
```
Therefore I've updated the original docs and added a large explanation to the
TSDoc for `page.evaluateHandle`.
In TypeScript land we'll assume the function will return a `JSHandle` but you
can tell TS otherwise via the generic argument, which can only be `JSHandle`
(the default) or `ElementHandle`:
```
const button = page.evaluateHandle<ElementHandle>(() => document.querySelector('button'));
```
jackfranklin
force-pushed
the
evaluate-handle-types
branch
from
3f585b5
to
9c9a59e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change started as a small change to pull types from DefinitelyTyped over to
Puppeteer for the
evaluateHandlefunction but instead ended up also fixingwhat looks to be a long standing issue with our existing documentation.
evaluateHandlecan in fact return anElementHandlerather than aJSElementif you return something that is an HTML element:
Therefore I've updated the original docs and added a large explanation to the
TSDoc for
page.evaluateHandle.In TypeScript land we'll assume the function will return a
JSHandlebut youcan tell TS otherwise via the generic argument, which can only be
JSHandle(the default) or
ElementHandle:This is part of #6124