shaka-player icon indicating copy to clipboard operation
shaka-player copied to clipboard

Generate TypeScript typings

Open mad-gooze opened this issue 7 years ago • 57 comments

Hello!

I would like to use shaka with a typescript project, but there are no typings. I have tried to generate d.ts using clutz, but did not manage to get any useful results. Are there any plans for typescript support?

mad-gooze avatar Sep 21 '17 17:09 mad-gooze

We looked into switching over to Typescript before, but decided to stick with Closure. I'll have to consult with Joey about adding Typescript support. If we have to maintain declarations files in addition to our existing externs, it'll probably add a lot of maintenance, so we'll see.

theodab avatar Sep 21 '17 20:09 theodab

You could have a look at extending our closure extern generator to also generate typescript files as well. You'll find this in the build folder.

We don't have time to work on this right now, but we would welcome a contribution if you would like to work on this sooner than we can. For now, I'm adding this to the backlog.

joeyparrish avatar Sep 21 '17 22:09 joeyparrish

Would incorporating clutz be an option? I already wanted to try building TypeScript bindings for shaka-player using the clutz tool, but unfortunately I couldn't compile clutz successfully on neither macOS 13 nor Windows 10.

niklaskorz avatar Jan 28 '18 14:01 niklaskorz

We don't have any experience with clutz, so it's hard to say. This is currently on our backlog, so we are not working on it right now.

joeyparrish avatar Jan 29 '18 18:01 joeyparrish

Thanks for the quick reply and the magnificent work you are doing on shaka-player. 💯

niklaskorz avatar Jan 29 '18 18:01 niklaskorz

So I have spent about a day writing type definitions for all exported shaka-player classes, interfaces, events and functions. It's complete and based on my understanding of both the shaka-player API docs and the TypeScript language. Doc comments are currently missing, but I'm considering to spent a bit of time there as well, copying the descriptions from the official API docs.

I would be happy to provide a pull request, but I also understand that the shaka-player team may prefer an automated solution to a handwritten one. If this is the case and you do not want to officially support a handwritten version, I'm going to open a pull request on the DefinitelyTyped repository so us TypeScript-users can make use of these until officially supported typings exist.

niklaskorz avatar Feb 01 '18 13:02 niklaskorz

Yes, an automated solution would be much preferred. Humans are generally lazy (like me!) so hand-written typings are likely to get out of sync quickly as the API expands and evolves.

We already have automation for the Closure-equivalent. Please see build/generateExterns.js. If you could extend this to generate typescript typings, or if you could generate typescript typings from the output of this tool (dist/shaka-player.compiled.externs.js), that would be ideal.

Our existing tool for Closure externs is written entirely in nodejs, and uses esprima to parse JavaScript and generate output based on the abstract syntax tree.

Are you interested in contributing to typescript automation?

joeyparrish avatar Feb 01 '18 15:02 joeyparrish

Alright!

Definitely interested, I'll have a look in the coming days.

niklaskorz avatar Feb 01 '18 15:02 niklaskorz

So I have spent a bit of time figuring out how the closure externs generator works and I am starting to get an idea of how the TypeScript generator could work. Will get back to you once I have something working. 😄

niklaskorz avatar Feb 02 '18 08:02 niklaskorz

Thanks!

joeyparrish avatar Feb 02 '18 19:02 joeyparrish

Slowly getting there.

The result of a day's work: https://gist.github.com/niklaskorz/6ac5d1917024033e8b050afbd9677d68

Obviously quite a few things are still missing, like the types themselves (need to finish the correct formatting for that) or interfaces.

niklaskorz avatar Feb 03 '18 01:02 niklaskorz

Awesome!

joeyparrish avatar Feb 03 '18 01:02 joeyparrish

@niklaskorz Super excited about your work!! I have had a look at your gist and found that you are currently emitting namespaces / internal modules. Are you planning to generate external module definitions, too?

yamass avatar Mar 01 '18 09:03 yamass

@yamass The problem is that shaka-player doesn't export its internal submodules as modules, they are only available as namespaces on the exported shaka object. If shaka-player happens to generate non-UMD modules in the future (i.e. ESNext or CommonJS), I see no problem in adding support for that to the typings generator.

niklaskorz avatar Mar 01 '18 12:03 niklaskorz

any news about this?

FrancescoBorzi avatar Oct 19 '18 12:10 FrancescoBorzi

just in case, if someone is interested, there is a lite update of the original gist here: https://gist.github.com/sco974/9c72ded8fe3e2e32ad2c2e41804ce642

tonton-sco-en-fr avatar Dec 13 '18 15:12 tonton-sco-en-fr

@sco974 Shall we add this definition to DefinitelyTyped and publish a package @types/shaka-player before Shaka team decides to maintain ts definition? what do you think?

iplus26 avatar Jul 21 '20 06:07 iplus26

Hello @iplus26, up to you, do what you think is the best :-) certainly this gist won't be useful anymore in some time, but it may be good to have a kind of backup ...

tonton-sco-en-fr avatar Jul 21 '20 07:07 tonton-sco-en-fr

@sco974 @iplus26 If there's interest, I can look into providing an up to date version for shaka player v3 based on my generator and submit it to @types. Best case though I'll be able to make it not only run successful (i.e., without fatal errors) on the current state of the repository but also have the output correct again.

niklaskorz avatar Jul 21 '20 09:07 niklaskorz

@niklaskorz Great. Pls generate a base version and I may do some additional work (like formatting or renaming interfaces) if needed.

iplus26 avatar Jul 21 '20 10:07 iplus26

Though Clutz didn't work for me in the past, I tried it again today and found it's working better now. The output isn't perfect, but it isn't too difficult to clean up with a few regex.

We don't have any TypeScript in our build system or our project, so we don't yet have a way to validate the output automatically. I've tried out the output in a few Google-internal projects that are TypeScript-based, and it's working in those contexts.

Can anyone offer a quick snippet of TypeScript that we can include for automatic verification of the generated TypeScript defs?

joeyparrish avatar Oct 07 '20 21:10 joeyparrish

Very nice @joeyparrish ! Clutz didn't work for me two years ago either, good to see that's changed. My repository includes some typescript samples for validating the typings: https://github.com/niklaskorz/shaka-player/tree/feature/generate-typescript-typedefs/test/typescript

niklaskorz avatar Oct 08 '20 07:10 niklaskorz

Thank you! I'll take a look.

joeyparrish avatar Oct 08 '20 16:10 joeyparrish

Hey @joeyparrish, we upgraded to v3.0.6 to get the integrated declarations instead of having ours.

First thing I have to say is that Clutz now is archived, so I don't think this is a good idea to use it, even if there's no other alternative I guess.

Second, I'm going to write down here few problems we met while using integrated declarations. In the end of the message, I'm going to attach you my declarations file.


We found that 3.0.6 typings misses few interfaces we use, for example shaka.Player.BufferingEvent, which can be used to identify parameters of functions, like this:

this.player = new shaka.Player(videoNode);
this.player.addEventListener('buffering', onShakaBuffering_);

// Somewhere, over the rainbow...

function onShakaBuffering(event: shaka.Player.BufferingEvent) {
    	let buffering = e.buffering;
	console.log('SHAKA - BUFFERING: ' + buffering);
	this.changePlaybackState(buffering ? 'BUFFERING' : 'READY');
}

In fact, when I wrote Shaka player typings starting from the documentation and the code, I created (manually) a file of over 1800 LOC for almost everything. And in these lines I defined BufferingEvent like this below. The file is attached at the end of the message.

		interface BufferingEvent extends Event {
			type: 'buffering';
			/**
			 * True when the Player enters the
			 * buffering state. False when the
			 * Player leaves the buffering state.
			 */
			buffering: boolean;
		}

Also some other interfaces are not exported, like google.ima.dai.api.StreamRequest. This happens when compiling through tsc.

> ../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:446:62 - error TS2694: Namespace 'google.ima.dai.api' has no exported member 'StreamRequest'.

446     requestServerSideStream (imaRequest : google.ima.dai.api.StreamRequest , backupUrl ? : string ) : Promise <string>;
                                                                                                                ~~~~~~~~~~~~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:574:37 - error TS2694: Namespace 'google.ima' has no exported member 'Ad'.

574     constructor (imaAd : google.ima.Ad , imaAdManager : google.ima.AdsManager ) ;
                                                                   ~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:598:45 - error TS2694: Namespace 'google.ima.dai.api' has no exported member 'Ad'.

598     constructor (imaAd : google.ima.dai.api.Ad | null , video : HTMLMediaElement | null ) ;
                                                                               ~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2343:41 - error TS2694: Namespace 'google.ima' has no exported member 'AdDisplayContainer'.

2343     constructor (container : google.ima.AdDisplayContainer ) ;
                                                                       ~~~~~~~~~~~~~~~~~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2347:34 - error TS2694: Namespace 'google.ima' has no exported member 'ImaSdkSettings'.

2347     getSettings ( ) : google.ima.ImaSdkSettings | null ;
                                                           ~~~~~~~~~~~~~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2417:130 - error TS2694: Namespace 'google.ima.dai.api' has no exported member 'UiSettings'.

2417     constructor (videoElement : HTMLMediaElement | null , adUiElement ? : HTMLElement | null , uiSettings ? : google.ima.dai.api.UiSettings | null ) ;
                             ~~~~~~~~~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2426:55 - error TS2694: Namespace 'google.ima.dai.api' has no exported member 'StreamRequest'.

2426     requestStream (streamRequest : google.ima.dai.api.StreamRequest | null ) : any ;
                                                                                                    ~~~~~~~~~~~~~

../../node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2765:62 - error TS2694: Namespace 'google.ima.dai.api' has no exported member 'StreamRequest'.

2765     requestServerSideStream (imaRequest : google.ima.dai.api.StreamRequest , backupUrl ? : string ) : Promise <string>;
                                                                                                                 ~~~~~~~~~~~~~

Is is possible to match the existing types somehow? I never used Clutz, so I'm not sure.


Also we face another error related to imports, maybe related to the fact that we are using Webpack to bundle and, consequentially, the fact that we have to use Common JS imports.

src/renderer.tsx:2:24 - error TS2306: File '<stripped path>/node_modules/shaka-player/dist/shaka-player.compiled.d.ts' is not a module.

2 import * as shaka from 'shaka-player';
                                       ~~~~~~~~~~~~~~

I've created a CodeSandbox to show it to you. In this sandbox, I added an override to package declarations and I created two files with these two different shaka import.

import * as shaka from "shaka-player";
import shaka from "shaka-player";

Now, the first import should be creating already a namespace while the second should use ESM default.

I did some researches and found that by adding the following statement to the bottom of declaration file, so we can mark it as a module:

export = shaka;

This is needed for CommonJS and AMD modules and it also allows typescript to recognize the syntax:

import shaka = require("shaka-player");

But this is unsupported in the CodeSandbox I created due to babel typescript transpiler in the default sandbox setup (I should setup Webpack there, but it have no sense for an example), but typescript supports it.

Can you add this line please?


For the two issues above, we had to override the typings as I did in the CodeSandbox, but we had few problems due to transpiling and bundling phases because we had to create another tsconfig.json.

So, I'm going to attach you our declaration file that I created 2 years ago, for out projects that use shaka-player, and kept updating it when I found something wrong. It might not be fully-correct, but it always worked for us (starting since we begun using shaka-player two years ago).

If it is not possible to create reliable and complete declarations for shaka-player, I'd embrace the idea suggested above and create and submit a types repo to DefinitelyTyped. I already have contributed to public typings so it would not be an issue for me to create such package.

They would be human-updated but easier to maintain for me.

shaka.d.ts.zip

Hope this helps and I hope to not have committed any mistake in this 😄.

alexandercerutti avatar Nov 19 '20 18:11 alexandercerutti

I can, unfortunately, confirm that the Clutz typings are not working as they are not exporting anything. Getting the same error as @alexandercerutti: File 'node_modules/shaka-player/dist/shaka-player.compiled.d.ts' is not a module. ts(2306)

Edit: I got it to work as @alexandercerutti described by loading an additional custom type declaration:

declare module 'shaka-player' {
  export = shaka;
}

Event types are still missing. In particular:

  • shaka.Player.BufferingEvent
  • shaka.Player.ErrorEvent

Edit 2: I'm confident this can be fixed by some handwritten additions, not sure how those would be integrated in the clutz workflow though. A particularly tricky part is that multiple addEventListener overloads will have to be added to the Player class declaration as well, to automatically get the correct types for event objects.

niklaskorz avatar Nov 19 '20 22:11 niklaskorz

@niklaskorz I was thinking the same about addEventListener overloads, and I don't think that an automated system, that is not typescript itself which allows the overloads already in the code, could create them as detailed way as a human could do. I don't know if there are better systems to generate declarations. But let me say that creating typings is something pretty challenging... and I love to create them! 😂

Anyway, there are a lot more Event structures and others that are missing other than those two.

alexandercerutti avatar Nov 20 '20 00:11 alexandercerutti

Another issue I just encountered is that shaka-player's semi complete Google IMA typings clash with our own complete Google IMA typings. Not sure yet how this could be avoided.

Edit: The type clash was caused by both shaka-player and our typings declaring namespaces in the global scope. I resolved this by moving our own declarations to a module level so they had to be imported. That way, the explicitly imported types aren't confused with shaka-player's global typings: https://github.com/alugha/typed-ima-sdk/commit/0cc8564e3f305cf01c3226767c6eb100799c0bcc

niklaskorz avatar Nov 20 '20 07:11 niklaskorz

Adding import 'shaka-player/dist/shaka-player.ui'; in one TS file allow importing typings without the need to add export = shaka;. Compilation and completion is working that way without the need to add something like import * as shaka from "shaka-player";. Apparently it won't import the script though, so you had to had the script in index.html (or angular.json's script section in my case) to make it work at runtime or you will have a "shaka is not defined" error.

This is the way I found to make it work as is, I have not dig deeper to find another solution.

DDeis avatar Nov 20 '20 09:11 DDeis

I actually don't know how shaka-player-ui is meant to be used, I don't use it.

Types are actually not meant to be imported like this @DDeis. They should be available all together when importing shaka-player (or the package name / path).

This works because typescript uses import without from as a side-effect importing only. And since you are not using anything but the types from that file, typescript uses it for the types and exclude it in compilation phase I guess.

Moreover I'm noticing right now that shaka-player.ui.d.ts exports itself some of the declarations contained in the shaka-player.compiled.d.ts, like Player. They are kinda duplicated.

Only one file should be the source of truth for me. Or each file should contain what it exports if they can be imported all alone.

alexandercerutti avatar Nov 20 '20 10:11 alexandercerutti

@alexandercerutti, when using UI you only import shaka-ui not both shaka-player and shaka-ui. UI works the same way as shaka-player "standalone", except the import path, so from my comment the equivalent would be import 'shaka-player';.

I agree with you, this is just a workaround for people wanting to use 3.0.6 typings without modifying them. But I would also prefer to be able to do something like import shaka from 'shaka-player'.

Maybe import { Player } from 'shaka-player and import { Controls } from 'shaka-player-ui, would be nice, or have them with a "@shaka-player" scope (for instance @shaka-player/core, @shaka-player/ui, etc. or something similar). But that's another discussion.

DDeis avatar Nov 20 '20 11:11 DDeis