shaka-project / shaka-player

JavaScript player library / DASH & HLS client / MSE-EME player
Apache License 2.0
7.05k stars 1.33k forks source link

Generate TypeScript typings #1030

Open mad-gooze opened 6 years ago

mad-gooze commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.

niklaskorz commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

niklaskorz commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Alright!

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

niklaskorz commented 6 years ago

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 commented 6 years ago

Thanks!

niklaskorz commented 6 years ago

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 commented 6 years ago

Awesome!

yamass commented 6 years ago

@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 commented 6 years ago

@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 commented 5 years ago

any news about this?

tonton-sco-en-fr commented 5 years ago

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

iplus26 commented 4 years ago

@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 commented 4 years ago

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

niklaskorz commented 4 years ago

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

iplus26 commented 4 years ago

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

joeyparrish commented 3 years ago

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?

niklaskorz commented 3 years ago

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

joeyparrish commented 3 years ago

Thank you! I'll take a look.

alexandercerutti commented 3 years ago

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

niklaskorz commented 3 years ago

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:

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.

alexandercerutti commented 3 years ago

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

niklaskorz commented 3 years ago

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

DDeis commented 3 years ago

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.

alexandercerutti commented 3 years ago

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.

DDeis commented 3 years ago

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

alexandercerutti commented 3 years ago

@DDeis I was thinking the same about shaka-player scoping to create several packages. That would be a great thing for me, but it would imply big changes to the repository (but it would be great to have demo, player and ui as different packages) and yes, as you correctly said, this is another discussion. But if @joeyparrish will consider to pass to typescript in Shaka v4, this discussion could be applied.

joeyparrish commented 3 years ago

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.

:sob: I had no idea! Ugh...

I don't see any good alternative for now, but long-term we can do what we do to generate Closure compiler externs, which is that we parse the library's AST in node and generate the necessary output. This is exactly the plan I was trying to avoid by using Clutz.

We found that 3.0.6 typings misses few interfaces we use, for example shaka.Player.BufferingEvent

These are also missing from our generated Closure externs. It has to do with how we defined them in our sources:

/**
 * @event shaka.Player.BufferingEvent
 * @description Fired when the player's buffering state changes.
 * @property {string} type
 *   'buffering'
 * @property {boolean} buffering
 *   True when the Player enters the buffering state.
 *   False when the Player leaves the buffering state.
 * @exportDoc
 */

In short, this is only recognized by jsdoc, not the compiler. And since we don't generate externs for them, those externs never get converted into typescript defs.

Perhaps the best thing is to define actual types for our events.

Also some other interfaces are not exported, like google.ima.dai.api.StreamRequest.

The situation with IMA APIs is a mess, even with Closure externs. We wrote our own externs for IMA which, internally to Google, conflict with some proprietary ones. We will look into this.

we can mark it as a module:

export = shaka;

Can you add this line please?

Yes, definitely!

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.

I hear you, but what we're trying to do is avoid human-updated defs. Manually-updated definitions, even with dedicated volunteers like yourself, will never keep up with new changes in the long run. That's why we're investing in automation. This was our first release with these, and they are not quite right. We'll fix them, and we're grateful for the feedback.

One of the things we need to add (that would have caught some of these issues) is a small typescript sample that gets compiled against these definitions as part of the build process. That way, if the generated definitions are broken, we know earlier, before release. And if something is missing, like BufferingEvent, we can add a reference to the TS sample code to make sure we don't regress.

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.

This is because those are for two different builds of the library. One includes the UI, and one does not. You should use whichever one you need, but never both.

We never wanted to force anyone to use our UI, nor to make the end-user spend extra time downloading code the app won't use. So ".compiled" is the traditional, UI-less build, and ".ui" is the build that includes everything. We originally intended to take a layered approach where the UI library would enhance the basic library, but we struggled to cleanly separate them because of compiler issues. I've forgotten the details since, but here we are. Ideally, some future, more modular version of Shaka Player would solve this and other issues by letting you import the specific pieces you want.

But if @joeyparrish will consider to pass to typescript in Shaka v4...

We can't make any promises at this point in time, but we have evaluated conversion to TypeScript in the past. I'm sure we will evaluate it again in the future. If/when we decide to move away from JavaScript and the Closure compiler, we will be sure to make a big announcement.

alexandercerutti commented 3 years ago

Thank you for your very detailed reply.

TSC also allows parsing Javascript and JSDoc, and it is able to generate declaration files from JSDOC. I think you might want to give it a try to generate .d.tss before making an attempt in any work (that might result in an overcomplicated work) to probably achieve the same result.

Let me know!


I hear you, but what we're trying to do is avoid human-updated defs. Manually-updated definitions, even with dedicated volunteers like yourself, will never keep up with new changes in the long run.

Well, at least you care about this. Many many other developers do not even try to care about typescript declarations. 😒


image

caridley commented 3 years ago

We also have a strong interest in using Typescript. In the absence of a solid automated process for generating the types we would be happy to help with manually generated types. I think it would be better to have a single typing in the shaka project than to have multiple users maintaining their own types. If types are added we would like to see them in the 2.5.x and 3.0.x branches.

@alexandercerutti I tried to use the jsdoc to ts.d process https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html using this command npx typescript lib/**/*.js --declaration --allowJs --emitDeclarationOnly --outDir types this produced a tree of .ts.d files that mirrored the lib directory, but they were all empty.

caridley commented 3 years ago

@alexandercerutti the types you provided provided in https://github.com/google/shaka-player/files/5568831/shaka.d.ts.zip seem to work reasonably well.

alexandercerutti commented 3 years ago

@caridley thank you, it took me a bit of time to create them manually (I've been working with shaka player for over two years now). Anyway, I didn't have the chance to test typescript declarations based on JSDoc, so I don't know how effective could it be as a solution.

caridley commented 3 years ago

@alexandercerutti Here is an updated version of your type file in which I have added an event map for addEventListener

shaka-with-addEventListener.d.ts.zip

Note that I reformatted turning tabs to spaces, added some eslint directive to make it work with our build process.

dave1981 commented 3 years ago

This type file should be extended to include also classes, methods and events related to ads, is it correct? https://shaka-player-demo.appspot.com/docs/api/tutorial-ad_monetization.html

RyanBellPOST commented 3 years ago

Any updates on getting a types definition file for the shaka player?

joeyparrish commented 3 years ago

We haven't had time to finish this work. The generated types still have some issues.

Here is what I am aware of that needs to be done to complete the work:

  1. Create real types in Closure for Shaka events (currently just documented in jsdoc without being "real" to the compiler). These will then be translated into generated TypeScript types by the existing tools.
  2. Add automated tests to ensure the generated types are complete and usable. This requires a sample app in TypeScript which is complete enough to find any issues in the generated types.
  3. Sync externs/types for the IMA SDK to avoid conflicts with other definitions.

There may be other issues, but these are the ones I know about.

mseeley commented 3 years ago

Pulled shaka-player 3.1.0 into a new TS project today. It may be a peculiarity of our environment but, the only way I was able to have the generated typings active was to duplicate shaka-player.compiled.d.ts to our typings directory then add the following to the document:

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

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

Afterwards I copied shaka-player.copiled.debug.d.ts to our typings directory too then added the following:

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

Thanks for providing these up-to-date generated typings until final ones are worked out. 👍

jbreemhaar commented 3 years ago
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!

benbro commented 2 years ago

Does this help with https://github.com/google/shaka-player/issues/3185?

Bec-k commented 2 years ago

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

IsaacSNK commented 2 years ago

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

lcaprini commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago
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 commented 1 year ago

@mseeley @jbreemhaar thanks a lot!