chore: migrate src/Browser to TS #5761
Conversation
This was an interestng migration and hit upon some tweaks elsewhere in the codebase where we can gain stricter typing now more code is ported to TS.
jackfranklin
force-pushed
the
migrate-browser-ts
branch
from
03e1292
to
4c6a064
Compare
| ['midi', 'midi'], | ||
| ['notifications', 'notifications'], | ||
| // TODO: push isn't a valid type? | ||
| // ['push', 'push'], |
There was a problem hiding this comment.
@mathiasbynens this is interesting to me. The protocol doesn't list push as a valid permission type so I'm not quite sure if:
- the protocol is wrong
- Puppeteer is wrong and calling overridePermissions with
pushwas effectively a no-op.
I think I'm lacking context here so I can't really make a call. We can keep the code and loosen the types to string[] rather than PermissionType[] but if this is highlighting a bug it's worth exploring.
There was a problem hiding this comment.
I lack the context here as well. Let's stick with the existing behavior for now, and we can resolve this separately later. Thanks for adding the TODO.
| @@ -72,7 +72,7 @@ export class Connection extends EventEmitter { | |||
| return this._url; | |||
| } | |||
|
|
|||
| send<ReturnType extends {}>(method: string, params = {}): Promise<ReturnType> { | |||
| send<T extends keyof Protocol.CommandParameters>(method: T, params?: Protocol.CommandParameters[T]): Promise<Protocol.CommandReturnValues[T]> { | |||
There was a problem hiding this comment.
this is an improvement on the types that I missed previously that was highlighted when migrating src/Browser
| @@ -19,7 +19,7 @@ | |||
| * to make this into TaskQueue<T> and let the caller tell us what types | |||
| * the promise in the queue should return. | |||
| */ | |||
| class TaskQueue { | |||
| export class TaskQueue { | |||
There was a problem hiding this comment.
this change allows the file to be imported into other TS files nicely
This was an interestng migration and hit upon some tweaks elsewhere in
the codebase where we can gain stricter typing now more code is ported
to TS.