Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate TypeScript typings #1030

Open
mad-gooze opened this issue Sep 21, 2017 · 56 comments
Open

Generate TypeScript typings #1030

mad-gooze opened this issue Sep 21, 2017 · 56 comments
Assignees
Labels
priority: P3 Useful but not urgent type: enhancement New feature or request
Milestone

Comments

@mad-gooze
Copy link
Contributor

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?

@theodab theodab added needs triage type: enhancement New feature or request and removed needs triage labels Sep 21, 2017
@theodab
Copy link
Contributor

theodab commented Sep 21, 2017

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.

@joeyparrish
Copy link
Member

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 joeyparrish added this to the Backlog milestone Sep 21, 2017
@joeyparrish joeyparrish added the flag: seeking PR We are actively seeking PRs for this; we do not currently expect the core team will resolve this label Sep 21, 2017
@joeyparrish joeyparrish modified the milestones: Backlog, v2.4.0 Oct 8, 2017
@joeyparrish joeyparrish modified the milestones: v2.4.0, Backlog Dec 4, 2017
@niklaskorz
Copy link
Contributor

niklaskorz commented Jan 28, 2018

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.

@joeyparrish
Copy link
Member

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.

@niklaskorz
Copy link
Contributor

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

@niklaskorz
Copy link
Contributor

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.

@joeyparrish
Copy link
Member

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?

@niklaskorz
Copy link
Contributor

Alright!

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

@niklaskorz
Copy link
Contributor

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. 😄

@joeyparrish
Copy link
Member

Thanks!

@niklaskorz
Copy link
Contributor

niklaskorz commented Feb 3, 2018

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.

@joeyparrish
Copy link
Member

Awesome!

@yamass
Copy link

yamass commented Mar 1, 2018

@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?

@niklaskorz
Copy link
Contributor

@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.

@FrancescoBorzi
Copy link

any news about this?

@tonton-sco-en-fr
Copy link

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

@joeyparrish joeyparrish modified the milestones: Backlog, Backlog 2 Jan 28, 2020
@iplus26
Copy link

iplus26 commented Jul 21, 2020

@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?

@tonton-sco-en-fr
Copy link

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 ...

@joeyparrish joeyparrish modified the milestones: v3.2, v3.3 Jul 7, 2021
@jbreemhaar
Copy link

jbreemhaar commented Jul 16, 2021

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

declare module 'shaka-player/dist/shaka-player.compiled' {
  export = shaka;
}

I guess the 'types' property is missing in the shaka-player package.json?
I didn't have to duplicate the files though, after adding the above to a custom type declaration it all works 👍
Thanks!

@joeyparrish joeyparrish removed the gsoc label Sep 27, 2021
@theodab theodab added the priority: P3 Useful but not urgent label Sep 30, 2021
@benbro
Copy link

benbro commented Oct 17, 2021

Does this help with #3185?

@Bec-k
Copy link

Bec-k commented Nov 22, 2021

Was surprised that shaka-player doesn't have types. Any updates on this? Woah more than 3 years have passed...

@IsaacSNK
Copy link
Contributor

IsaacSNK commented Apr 20, 2022

@caridley I gave tsc a try to generate typings from JSDoc. Seems like declaring classes with expressions breaks the type generation. For example, the following code

/**
 * @summary The main player object for Shaka Player.
 *
 * @implements {shaka.util.IDestroyable}
 * @export
 */
Player = class  {
  /**
   * @param {HTMLMediaElement=} mediaElement
   *    When provided, the player will attach to <code>mediaElement</code>,
   *    similar to calling <code>attach</code>. When not provided, the player
   *    will remain detached.
   * @param {function(Player)=} dependencyInjector Optional callback
   *   which is called to inject mocks into the Player.  Used for testing.
   */
  constructor(mediaElement, dependencyInjector) {
    super();
  }
}

generates an empty .d.ts. But the following code

/**
 * @summary The main player object for Shaka Player.
 *
 * @implements {shaka.util.IDestroyable}
 * @export
 */
class Player  {
  /**
   * @param {HTMLMediaElement=} mediaElement
   *    When provided, the player will attach to <code>mediaElement</code>,
   *    similar to calling <code>attach</code>. When not provided, the player
   *    will remain detached.
   * @param {function(Player)=} dependencyInjector Optional callback
   *   which is called to inject mocks into the Player.  Used for testing.
   */
  constructor(mediaElement, dependencyInjector) {
    super();
  }
}

generates the following type

/**
 * @summary The main player object for Shaka Player.
 *
 * @implements {shaka.util.IDestroyable}
 * @export
 */
declare class Player implements shaka.util.IDestroyable {
    /**
     * @param {HTMLMediaElement=} mediaElement
     *    When provided, the player will attach to <code>mediaElement</code>,
     *    similar to calling <code>attach</code>. When not provided, the player
     *    will remain detached.
     * @param {function(Player)=} dependencyInjector Optional callback
     *   which is called to inject mocks into the Player.  Used for testing.
     */
    constructor(mediaElement?: HTMLMediaElement | undefined, dependencyInjector?: ((arg0: Player) => any) | undefined);
}

Are class expressions required for Closure? Is there any restriction on using ES6 Modules along with Closure? I'm not familiar with Closure compiler.

@avelad avelad modified the milestones: v3.3, v4.1 May 4, 2022
@avelad avelad modified the milestones: v4.1, v4.2 Jun 3, 2022
@avelad avelad modified the milestones: v4.2, v4.3 Aug 17, 2022
@joeyparrish joeyparrish modified the milestones: v4.3, Backlog Sep 21, 2022
@lcaprini
Copy link

Hi all. A good improvement of that could be done on the shaka.extern.PlayerConfiguration and all its nested definitions, because actually all of them are mandatory.
In my project I would like to edit only some of them like

const configObject: shaka.extern.PlayerConfiguration = {
  manifest: {
    disableVideo: true,
    hls: {
      useSafariBehaviorForLive: false,
    },
  },
};

but the Type '{ useSafariBehaviorForLive: false; }' is missing the following properties from type 'HlsManifestConfiguration': defaultAudioCodec, defaultVideoCodec, ignoreImageStreamFailures, ignoreManifestProgramDateTime, and 3 more.ts(2740) error occurs.

Of course shaka.extern.PlayerConfiguration and all its nested object have the same behaviour.

Thanks.

@joeyparrish
Copy link
Member

@IsaacSNK wrote:

Are class expressions required for Closure? Is there any restriction on using ES6 Modules along with Closure? I'm not familiar with Closure compiler.

They are not required, but that is how it works to add them to a global namespace. That will change as we move to modules and TypeScript. (Likely in that order.)

@lcaprini wrote:

Hi all. A good improvement of that could be done on the shaka.extern.PlayerConfiguration and all its nested definitions, because actually all of them are mandatory.

They are, but the configure() method doesn't technically take a full PlayerConfiguration. It takes an Object with any subset of that, and it merges the input with its full PlayerConfiguration. I see that this is confusing, though.

@lcaprini
Copy link

lcaprini commented Dec 14, 2022

@IsaacSNK wrote:

They are, but the configure() method doesn't technically take a full PlayerConfiguration. It takes an Object with any subset of that, and it merges the input with its full PlayerConfiguration. I see that this is confusing, though.

Hi @IsaacSNK , I saw the object in configure()'s type definition, and actually I'm using it in that way; by the way I completely loose the IDE intellisense, that could be very helpful.

JSDoc supports the optional member using ?, so using it in all PlayerConfiguration members (and all its nested object of course) should be fix the "issue".

A different way could be using the Partial<...> on all definitions, but I don't know if JSDoc supports it. ie:

from

declare namespace shaka.extern {
  type HlsManifestConfiguration = {
    defaultAudioCodec : string,
    defaultVideoCodec : string,
    ignoreImageStreamFailures : boolean,
    ignoreManifestProgramDateTime : boolean,
    ignoreTextStreamFailures : boolean,
    liveSegmentsDelay : number,
    mediaPlaylistFullMimeType : string,
    useSafariBehaviorForLive : boolean
  } ;
}

to

declare namespace shaka.extern {
  type HlsManifestConfiguration = Partial<{
    defaultAudioCodec : string,
    defaultVideoCodec : string,
    ignoreImageStreamFailures : boolean,
    ignoreManifestProgramDateTime : boolean,
    ignoreTextStreamFailures : boolean,
    liveSegmentsDelay : number,
    mediaPlaylistFullMimeType : string,
    useSafariBehaviorForLive : boolean
  } >;
}
@Security2431
Copy link

error TS2306: File '/node_modules/shaka-player/dist/shaka-player.compiled.d.ts' is not a module

Types are generated but exports is not defined. So you can either remove types or follow up on my suggestions.

  1. Run npm i -D patch-package.
  2. Download and move the shaka-player+4.3.4.patch file to /patches/shaka-player+4.3.4.patch in the root directory.
  3. Add "scripts": {"postinstall": "patch-package"} in package.json. This make patches to be applied each time people run npm install.
  4. Delete node_modules folder and execute npm install.

All files and step-by-step instructions can be found at the following link:
https://gist.github.com/Security2431/2b28f17e11870bb4b0e347673e16d5ba

@imtiaz101325
Copy link

@mseeley @jbreemhaar thanks a lot!

@falk-stefan
Copy link

This issue is referenced in the roadmap.md:


Candidate features for future release cycles:


@joeyparrish is there ever going to be proper TypeScript support? Applying a patch like #1030 (comment) suggests can't be the way to go .. 😅

@joeyparrish
Copy link
Member

I haven't been able to plan exactly when, but I hope to convert the whole project to TypeScript this year. I worry it will be very painful, though. And I need to finish some other projects first, in any case.

@DarrinGinderPost
Copy link

Any update on this? My team is hoping to transition but we are a typescript environment.

@falk-stefan
Copy link

I'm stuck with v4.3.4 - any chance we'll see this any time soon?

@joeyparrish
Copy link
Member

We (Shaka team within Google) will probably not do this until Q1 2025.

@falk-stefan, why are you stuck? Why would upgrading be an issue?

@alexandercerutti
Copy link
Contributor

alexandercerutti commented Sep 1, 2024

@joeyparrish I'd like to contribute when you are going to move to Typescript. Transition to Typescript can also be performed one file at time, to not distrupt everything, as Typescript supports a subset of JSDoc (TSDoc) and has two options called allowJS and checkJS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: P3 Useful but not urgent type: enhancement New feature or request