feat: make JSHandle generic #140
Conversation
| const resolved = await this.resolveSelector(selector); | ||
| const handle = await this.context.evaluateHandle( | ||
| (injected: Injected, selector: string, scope: SelectorRoot | undefined, visible: boolean | undefined) => { | ||
| const element = injected.querySelector(selector, scope || document); | ||
| (injected: Injected, selector: string, scope?: Node, visible?: boolean) => { |
There was a problem hiding this comment.
Why scope type has changed? Injected cannot query against a Node, only against some specific types (SelectorRoot).
There was a problem hiding this comment.
It's true that injected can only query against certain kinds of Nodes, but it is prepared to accept and throw the appropriate error if it is wrong.
We could have ElementHandle check that the Node is a a SelectorRoot, but that would take an extra round trip. So I changed the type of scope to accurately reflect its value.
| @@ -144,7 +144,7 @@ export class DOMWorld { | |||
| } | |||
| } | |||
|
|
|||
| export class ElementHandle extends js.JSHandle { | |||
| export class ElementHandle<T extends Node = Node> extends js.JSHandle<T> { | |||
There was a problem hiding this comment.
Let's make the default Element? This will simplify a lot of places.
There was a problem hiding this comment.
It causes us to need more ElementHandle<Node> instead.
src/injected/injected.ts
Outdated
| const parsed = this._parseSelector(selector); | ||
| let element = root; | ||
| let element = root as Element; |
There was a problem hiding this comment.
This does not seem to be true.
There was a problem hiding this comment.
made it check the type and throw an error before continuing
| export type EvaluateOn = <Args extends any[], R>(pageFunction: PageFunctionOn<any, Args, R>, ...args: Boxed<Args>) => Promise<R>; | ||
| export type EvaluateHandleOn = <Args extends any[]>(pageFunction: PageFunctionOn<any, Args>, ...args: Boxed<Args>) => Promise<js.JSHandle>; | ||
| export type EvaluateHandle = <Args extends any[], R>(pageFunction: PageFunction<Args, R>, ...args: Boxed<Args>) => Promise<Handle<R>>; | ||
| export type $Eval<O = string | Selector> = <Args extends any[], R, S extends O>(selector: S, pageFunction: PageFunctionOn<ElementForSelector<S>, Args, R>, ...args: Boxed<Args>) => Promise<R>; |
There was a problem hiding this comment.
O is the allowed options, S is the specific string or type that got passed it. It lets me do $eval('input', x => ...) and know that x is HTMLInputElement because S is 'input' not string
src/dom.ts
Outdated
| @@ -242,13 +246,13 @@ export class ElementHandle extends js.JSHandle { | |||
| } | |||
|
|
|||
| async setInputFiles(...files: (string|input.FilePayload)[]) { | |||
| const multiple = await this.evaluate((element: HTMLInputElement) => !!element.multiple); | |||
| const multiple = await this.evaluate((element: Node) => !!(element as HTMLInputElement).multiple); | |||
src/dom.ts
Outdated
| assert(multiple || files.length <= 1, 'Non-multiple file input can only accept single file!'); | ||
| await this._world.delegate.setInputFiles(this, await input.loadFiles(files)); | ||
| } | ||
|
|
||
| async focus() { | ||
| await this.evaluate(element => element.focus()); | ||
| await this.evaluate((element: Node) => (element as HTMLElement).focus()); |
| @@ -76,7 +76,7 @@ export class JSHandle { | |||
| return this._context._delegate.getProperties(this); | |||
| } | |||
|
|
|||
| jsonValue(): Promise<any> { | |||
| jsonValue(): Promise<T> { | |||
There was a problem hiding this comment.
Let's type the ExecutionContextDelegate.
There was a problem hiding this comment.
I typed handleJSONValue, but getProperties isn't great to type. You can't make a Map in typescript that has specific types of keys go to specific types of values. We could do it if we were returning plain objects, but maps is a no-go.
This makes it so that JSHandles and ElementHandles are aware of what types they point to. As a fun bonus,
$eval('input')knows its going to get an HTMLInputElement.Most of this patch is casting things where previously we just assumed ElementHandles held the right kind of node. This gets us closer to being able to turn on
noImplicityAnyas well.#6