cyrus-and / chrome-remote-interface

Chrome Debugging Protocol interface for Node.js
MIT License
4.29k stars 309 forks source link

Add TypeScript Definition File(s) #112

Closed westy92 closed 5 years ago

westy92 commented 7 years ago

Create and publish. https://www.typescriptlang.org/docs/handbook/declaration-files/introduction.html

cyrus-and commented 7 years ago

Do you mean adding definition files for the library itself or for the protocol too?

In the former case it should be quite trivial as the exposed API is fairly small, in the latter I guess we'd need a way to generate it from the JSON during the prepublish phase, no idea about how hard it can be... But even in that case it wouldn't cover the case in which the protocol is fetched at runtime. So I don't think it's feasible. Thoughts?

PS I know approximately nothing about TypeScript.

westy92 commented 7 years ago

Just for the library itself.

paulirish commented 7 years ago

EDIT (July 2017): please see https://github.com/ChromeDevTools/devtools-protocol/issues/18.

the rest of my comment is mostly outdated.


I don't know of any current and maintained typings for the protocol except for the closure compiler externs that devtools team maintains in the protocol repo: https://github.com/ChromeDevTools/devtools-protocol/tree/master/externs

joelgriffith commented 7 years ago

I'd greatly benefit from this in my high-level project (https://github.com/joelgriffith/navalia). I'd be happy to tackle a Domain or two if manual work is needed.

Seems like a lot could be automated from the typedefs exposed inside of the protocol.

Bnaya commented 7 years ago

Looks like https://github.com/krisselden/chrome-debugging-client have very extensive ts interfaces for the protocol

cyrus-and commented 7 years ago

@joelgriffith, @Bnaya thanks for the info and willing to help. While there are certainly ways to generate the typing definitions from the JSON I don't think it would suit this module, the protocol.json bundled in this repo is there just for convenience, ideally one would pull it from Chrome directly (which is now possible), in this way one can be sure that the present version is the right one; or even pass in a custom JSON descriptor for whatever reason.

In these cases the generation wouldn't work because (AFAIK) the TypeScript definitions cannot be enforced at runtime. Rather, I think that this feature should be handled at user-level or by a third party (e.g., DefinitelyTyped) and used only when an application target a specific version of Chrome (unlikely) or if just a stable subset of the protocol is needed.

What do you think?

veryjos commented 7 years ago

TypeScript definitions are never really enforced during runtime, even with definitions generated directly from a TypeScript source.

TypeScript's compiler strives to essentially provide compile-time safety, so there's no reason to try to have runtime safety. It's not one of TypeScript's goals. Pragmatically, they try to stay as close to "JS with types" as possible. Compilation then essentially verifies the types, and strips all type information.

Could always just bundle a script to generate typings so a user can generate and utilize them for whatever version/configuration they'd like? Instead of maintaining a separate repository.

cyrus-and commented 7 years ago

Could always just bundle a script to generate typings so a user can generate and utilize them for whatever version/configuration they'd like? Instead of maintaining a separate repository.

@simply-jos this could actually make sense.

seantanly commented 7 years ago

Found the following which could be relevant for reference.

https://github.com/nojvek/chrome-remote-debug-protocol

paulirish commented 7 years ago

Canonical upstream issue is this: https://github.com/ChromeDevTools/devtools-protocol/issues/18

paambaati commented 7 years ago

@cyrus-and Would this work?

https://github.com/Microsoft/dts-gen

joelgriffith commented 7 years ago

Might be worth a shot using that script. It's better than nothing? Should we try and get it included in the @types namespace since that's where most TS definitions go that aren't part of their respective packages?

cyrus-and commented 6 years ago

Alright so let's try to recap, there are two separate parts of this issue:

  1. typing info for this module API, hand made;
  2. typing info for the protocol, generated somehow.

The problem with 2 is that it cannot be performed at runtime, meaning that the typings should ideally be re-generated for every release of Chrome. This is IMHO too cumbersome to be useful, but I understand that some people would benefit from that. Apparently something is moving (ChromeDevTools/devtools-protocol#90) so ideally the generation script will be hosted there; @paulirish can you confirm?

About 1, I agree on the DefinitelyTyped approach as @joelgriffith suggested. I will not embark on writing and maintaining a TypeScript interface for this module though, so whoever is interested should send a PR there. I'm of course available for any question about this module API.

paulirish commented 6 years ago

Yup, We'll be merging that protocol types PR once brendan and joel are happy.

The generation script will be part of the -protocol repo and the generated .d.ts will be distributed in the devtools-protocol npm package. (though I'm fine if someone else wants to ship them into a @types package or w/e)

On Tue, Mar 27, 2018 at 3:54 AM, Andrea Cardaci notifications@github.com wrote:

Alright so let's try to recap, there are two separate parts of this issue:

  1. typing info for this module API, hand made;
  2. typing info for the protocol, generated somehow.

The problem with 2 is that it cannot be performed at runtime, meaning that the typings should ideally be re-generated for every release of Chrome. This is IMHO too cumbersome to be useful, but I understand that some people would benefit from that. Apparently something is moving ( ChromeDevTools/devtools-protocol#90 https://github.com/ChromeDevTools/devtools-protocol/pull/90) so ideally the generation script will be hosted there; @paulirish https://github.com/paulirish can you confirm?

About 1, I agree on the DefinitelyTyped https://github.com/DefinitelyTyped/DefinitelyTyped approach as @joelgriffith https://github.com/joelgriffith suggested. I will not embark on writing and maintaining a TypeScript interface for this module though, so whoever is interested should send a PR there. I'm of course available for any question about this module API.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cyrus-and/chrome-remote-interface/issues/112#issuecomment-376482554, or mute the thread https://github.com/notifications/unsubscribe-auth/AACZF1eUhBtkTdcYkyvmYJbmzDU8OEtlks5tihprgaJpZM4NEjaO .

joelgriffith commented 6 years ago

Sounds like a plan!

piotrmoszkowicz commented 6 years ago

Hey ;) Is it possible to get it done finally :) ?

cyrus-and commented 6 years ago

@piotrmoszkowicz so:

paulirish commented 6 years ago

yah https://github.com/ChromeDevTools/devtools-protocol/tree/master/types probably has most of what you need, though you'll have to apply them a bit yourself.

paambaati commented 6 years ago

@cyrus-and @paulirish I'd like to take this up!

I already have rudimentary typings for all of CDP's built-in methods, but to import the protocol definitions, I'd either have to have a cron-like job that pulls in https://github.com/ChromeDevTools/devtools-protocol and adds it in and publishes it, or CDP can include this as a dependency and I can simply reference it.

The former is more tedious and requires an ever-running job, but keeps CDP clean. The latter is far lesser work, but includes an additional (albeit lightweight) dependency that non-TypeScript developers would not need.

Thoughts?

paulirish commented 6 years ago

@paambaati i'm open to adding the typings into the devtools-protocol package. wanna make a pull request there? you'll see the protocol-dts-generator.ts that generates the existing ones.

paambaati commented 6 years ago

@cyrus-and @paulirish Here's what I've written — https://github.com/paambaati/DefinitelyTyped/blob/chrome-remote-interface/types/chrome-remote-interface/index.d.ts

i'm open to adding the typings into the devtools-protocol package

@paulirish Do you mean adding typings for chrome-remote-interface into devtools-protocol? If yes, would that mean adding devtools-protocol as a dependency for TS users using this library?

cyrus-and commented 6 years ago

@paambaati did you submit the PR to DefinitelyTyped?

paambaati commented 6 years ago

@cyrus-and Not yet, I'm still mulling over how to add the Protocol methods, seeing as how they have to be auto-generated every time a new version drops.

@paulirish Bump!

Do you mean adding typings for chrome-remote-interface into devtools-protocol?

cyrus-and commented 5 years ago

So... there was an attempt (DefinitelyTyped/DefinitelyTyped#31313) but the PR has been closed for inactivity (?); those interested could try to resume that.

cyrus-and commented 5 years ago

I'm closing this as there isn't enough interest to make it happen apparently. Anyone interested can submit a PR to DefinitelyTyped possibly using DefinitelyTyped/DefinitelyTyped#31313 as a base line. (Feel free to link back here if you decide to do so.)

For what concerns the protocol API instead please refer to these files from ChromeDevTools/devtools-protocol.

Cheers.

kazarmy commented 3 years ago

I've submitted a PR for this at DefinitelyTyped (DefinitelyTyped/DefinitelyTyped#53913). The protocol API should be the latest from ChromeDevTools/devtools-protocol in my opinion, since that should cover all previous versions and also variants (strictly speaking this may not be true but it's the best approximation that I could find). I'm using a custom protocol-dts-generator.ts to generate the protocol API typings both to add Promise event methods and to get the typings past DefinitelyTyped's tslint, so it should be rather easy to update the protocol typings.

The chrome-remote-interface typings at DefinitelyTyped/DefinitelyTyped#53913 are currently incomplete though (tbh I only just noticed DefinitelyTyped/DefinitelyTyped#31313) but if all goes well I suppose they can be made complete with some work.

westy92 commented 3 years ago

I've done some work to extend the typings to cover the full functionality of chrome-remote-interface.

You can find them published here: https://www.npmjs.com/package/@types/chrome-remote-interface 🎉

cyrus-and commented 3 years ago

@westy92 thank you so much for this!

perpetuallight commented 3 years ago

@westy92 Are there special settings to get this to work? I have it installed with this module but I am not getting anything(ie. everything is typed as any)? I made sure that the proxy mappings were installed too. Do I need to npm install? Usually I just install the @types/module but I understand this situation is different. The other thing that may be quirky about my setup is that my vscode is telling me that I have to use "esModuleInterop": true, under compileroptions. and use the default export. Which now I see did put me back in touch with the API reality. But I still can't figure out how to traverse the protocol. I see it in the definition but not in my editor. What am I missing? I read the readme but I am not sure how to incorporate that method. Again I usually just install the @types/module and it works.

@cyrus-and How would you feel about updating the readme/docs to reflect this? I am sure it would make a world of difference. Sure we could tab back and forth between our code and docs. But who has time for that? ;)

I could do a PR once I figure out how to get it working.

cyrus-and commented 3 years ago

@cyrus-and How would you feel about updating the readme/docs to reflect this? I am sure it would make a world of difference.

@perpetuallight I'm OK with that as long as the instructions are kept succinct. I have no interest in TypeScript and I'm not willing to maintain something that gets obsolete at every version bump.

westy92 commented 3 years ago

@perpetuallight you can see how I used them in my own package here: https://github.com/westy92/html-pdf-chrome

I just did npm i @types/chrome-remote-interface, restarted vscode, and they worked for me.

perpetuallight commented 3 years ago

@westy92, Thanks for the tip. But I have actually been looking at this for the last couple days. I went ahead and tried restarting vscode to see if that helped but it didn't. There is actually a refresh button in the file explorer that I use. I think that also triggers the definitions to refresh if needed as well. That's my theory anyway.

I went to the repo to report my findings but it doesn't look like its a submodule. Should I just do it here? I'll go ahead and do it and create the issue over there if someone tells me I am wrong.

I did install globally and then link. Just tested it and it looks like undoing that and installing locally did make a difference.

But the original reason that I did it globally was that I was hoping there would be an easy way to refresh the definitions. I didn't find that. But I did notice that the devtools interface files were mis-matched with the CDP.Client. Which is kinda what I anticipated being a regular occurrence.

So my thought was that either the end user could(in an ideal world) either regen at will or select a source structure to generate from at install. Including straight from a local chrome instance /json/protocol endpoint. To cover all bases. For launching I hear that there is a google-launcher package that abstracts all platform concerns. Thoughts?

Also if you like my idea of installing globally and linking I was able to get it to work by fixing the path's in your index.d.ts that point to the ProtocolProxyApi and ProtocolMappings files. I just added './node_modules/' to the front. And the global linked setup worked correctly. But I see that for some reason the devtools-protocol dependency is not there when I install it locally. Not sure how it works without it locally and doesn't work with it globally linked. But maybe it should also work locally with it and the same path mod that I used globally linked. Theres a mouth full. (and to link it's npm link <module>)

Also I was going over the readme, here, more closely. And I saw that CDP.Client could expose a send method. But it wasn't defined in type CDP.Client. So I tried it for fun:

send<T extends keyof ProtocolMappingApi.Commands>(event: T, params: ProtocolMappingApi.Commands[T]["paramsType"][0], callback: (returnMessage: ProtocolMappingApi.Commands[T]["returnType"]) => void): void
send<T extends keyof ProtocolMappingApi.Commands>(event: T, params: ProtocolMappingApi.Commands[T]["paramsType"][0]): Promise<ProtocolMappingApi.Commands[T]["returnType"]>

Maybe not very useful for too many people. But it's there so maybe someone will use it if you want to include it.

perpetuallight commented 3 years ago

@perpetuallight I'm OK with that as long as the instructions are kept succinct. I have no interest in TypeScript and I'm not willing to maintain something that gets obsolete at every version bump.

I'll keep it restricted to stable examples similar to the non TypeScript examples already there.

Actually, now that I think about it... Implicit type sensing was working, including examples already in the readme, once I got everything situated correctly(ie. local install for now). So all that should be needed would be a mention of the install and import procedure. Everything else should be the same.

kazarmy commented 3 years ago

@perpetuallight Regarding https://github.com/cyrus-and/chrome-remote-interface/issues/112#issuecomment-893156481 it's somewhat unclear what your problem is, and it would be easier to help you if you were more precise, but I'm going to give it a try anyway.

As @westy92 suggests, the best way to keep full control over your type definitions is to install them locally via npm install. This command also provides you with an easy way to update the devtools-protocol dependency by using npm install devtools-protocol@latest. You can check package-lock.json to see what version of devtools-protocol is actually installed in node_modules/, if that isn't clear from your package.json. The devtools-protocol version is currently pinned (DefinitelyTyped/DefinitelyTyped#56223) so this won't work.

If you have your own /json/protocol and you want typings from it, you probably need to experiment with the https://github.com/ChromeDevTools/devtools-protocol/blob/master/scripts/protocol-dts-generator.ts script, but I think it's probably just a matter of making sure it's reading your json file instead of its own json files. To install your typings, like you state npm link should work, though I know there's also the option of using patch-package. Personally, I prefer the cleaner way of defining my own ambient TypeScript module using the following template:

declare module 'chrome-remote-interface' {
    // The following need skipLibCheck = true in tsconfig.json.
    import('./devtools-protocol/protocol-mapping');
    import('./devtools-protocol/protocol-proxy-api');
    import ProtocolMappingApi from './devtools-protocol/protocol-mapping';
    import ProtocolProxyApi from './devtools-protocol/protocol-proxy-api';

    /* eslint-disable capitalized-comments, max-len, quotes, spaced-comment */
    /* eslint-disable @typescript-eslint/no-explicit-any */

    // Copy and paste index.d.ts from DefinitelyTyped's chrome-remote-interface
    // here, starting from 'declare namespace CDP {' up to 'export = CDP;'
}

I put my custom protocol typings (perhaps the ones in https://github.com/kazarmy/devtools-protocol/tree/master/types) under the devtools-protocol subdirectory, set a link to the above typings in tsconfig.json, and it all works like a charm.

perpetuallight commented 3 years ago

@kazarmy the generator is exactly what I was looking for when I installed it "un-ideally". Thank you for pointing that out to me. I will definitely check it out.

But what I was indirectly suggesting was that maybe it could be included with the installable module. To give people the option to run it at or after install through a wizard like menu option(not sure if npm install supports that or not I've never seen it but it could be useful here, IMHO). If that interests the owners involved but no one has the time to tackle it I might be able to find time to look into it. Otherwise I won't bother. And with the install "wizard" option I am sure that a local install would still be ideal as you say.

Thanks again for the info.

kazarmy commented 3 years ago

If that interests the owners involved but no one has the time to tackle it I might be able to find time to look into it.

I'm okay with this. @westy92? I don't think DefinitelyTyped allows execution of arbitrary JavaScript but you might be able to include your wizard with chrome-remote-interface.

westy92 commented 3 years ago

I think we're over-engineering things here.

By default chrome-remote-interface asks the remote instance to provide its own protocol. Source

IMO that fact is what led the DefinitelyTyped definitions to use the latest version of the protocol.

If a user wants to use a custom version of the protocol, fine, but I think they should need to manually generate their own typings, if desired. I don't think baking in a custom tool is necessary, but maybe adding some documentation on how to do so would be helpful.

perpetuallight commented 3 years ago

Ok, sounds good to me. If DefinitelyTyped wouldn’t accept it I don’t think it would be worth redirecting someones focus back to the chrome-remote-interface for the “wizard”. Unless it included the type module automatically. Which I don’t think would be preferable in most cases. I’ll revisit adding documentation for custom versions once I figure it out for myself and I understand the use case better. I just have a few ideas for the use case that may or may not be universal.

On Aug 17, 2021, at 10:32 PM, Seth Westphal @.***> wrote:

I think we're over-engineering things here.

By default chrome-remote-interface asks the remote instance to provide its own protocol. Source https://github.com/cyrus-and/chrome-remote-interface#chrome-debugging-protocol-versions IMO that fact is what led the DefinitelyTyped definitions to use the latest version of the protocol.

If a user wants to use a custom version of the protocol, fine, but I think they should need to manually generate their own typings, if desired. I don't think baking in a custom tool is necessary, but maybe adding some documentation on how to do so would be helpful.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cyrus-and/chrome-remote-interface/issues/112#issuecomment-900823881, or unsubscribe https://github.com/notifications/unsubscribe-auth/AULVTY33JGTPDI6NULZ2MBTT5NAV7ANCNFSM4DISG2HA. Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email.