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

Shaka Typescript definition overriding Google Cast Sender with incomplete defs #3185

Closed
osmestad opened this issue Feb 26, 2021 · 21 comments · Fixed by #6936
Closed

Shaka Typescript definition overriding Google Cast Sender with incomplete defs #3185

osmestad opened this issue Feb 26, 2021 · 21 comments · Fixed by #6936
Assignees
Labels
priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly
Milestone

Comments

@osmestad
Copy link

osmestad commented Feb 26, 2021

Have you read the FAQ and checked for duplicate open issues?
Yes, this does probably relate to #1030, but I assume it is cleaner to report it as a separate bug.

What version of Shaka Player are you using?
3.0.8

Can you reproduce the issue with our latest release version?
NA

Can you reproduce the issue with the latest code from master?
Not tested

Are you using the demo app or your own custom app?
Own

If custom app, can you reproduce the issue using our demo app?
NA

What browser and OS are you using?
Any

For embedded devices (smart TVs, etc.), what model and firmware version are you using?
NA

What are the manifest and license server URIs?
NA

What did you do?
In code unrelated to the Shaka based player Typescript gives errors, example:

const sessionObj = session.getSessionObj();
sessionObj.addMediaListener(media => {
  onMediaSession(media);
});

What did you expect to happen?
Type from node_modules/@types/chrome/chrome-cast/index.d.ts to be resolved. (That is included when using the @types/chromecast-caf-sender package.)

What actually happened?
Type from node_modules/shaka-player/dist/shaka-player.compiled.d.ts seems to override, in this case:

// Generated from /usr/local/google/home/joeyparrish/hacking/shaka/clean/externs/chromecast.js
declare namespace chrome.cast {
  class Session {
    ...

Which does not contain the addMediaListener method.

I see there are similar issues reported for IMA types in #1030, maybe Shaka should not export these definitions for 'externs'? or maybe not in the global scope at least?

@joeyparrish
Copy link
Member

That makes sense. We should ideally not generate definitions for things that are not exposed by us. I don't believe the Cast API is exposed through ours.

The trouble is that some of the true external types that we depend on are exposed through our API, and without those definitions, the generated definitions for Shaka Player would be incomplete.

I'm not sure the right way to address this in the general case.

@joeyparrish joeyparrish added type: bug Something isn't working correctly and removed needs triage labels Mar 1, 2021
@joeyparrish joeyparrish self-assigned this Mar 1, 2021
@joeyparrish joeyparrish added this to the v3.2 milestone Mar 1, 2021
@NoChance777
Copy link
Contributor

I confirm this bug. Currently, we are on v3.0.5, and when I tried to upgrade to v3.0.8 got many errors about intersections with Chromecast types. If required I can provide more details

@NoChance777
Copy link
Contributor

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:599:45 - error TS2694: Namespace 'google.ima.dai.api' has no exported member 'Ad'.

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

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2269:9 - error TS2300: Duplicate identifier 'ApiConfig'.

2269   class ApiConfig {
             ~~~~~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:206:18
    206     export class ApiConfig {
                         ~~~~~~~~~
    'ApiConfig' was also declared here.

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2276:9 - error TS2300: Duplicate identifier 'Error'.

2276   class Error {
             ~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:232:18
    232     export class Error {
                         ~~~~~
    'Error' was also declared here.

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2286:9 - error TS2300: Duplicate identifier 'Receiver'.

2286   class Receiver {
             ~~~~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:452:18
    452     export class Receiver {
                         ~~~~~~~~
    'Receiver' was also declared here.

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2293:9 - error TS2300: Duplicate identifier 'Session'.

2293   class Session {
             ~~~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:300:18
    300     export class Session {
                         ~~~~~~~
    'Session' was also declared here.

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2309:9 - error TS2300: Duplicate identifier 'SessionRequest'.

2309   class SessionRequest {
             ~~~~~~~~~~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:280:18
    280     export class SessionRequest {
                         ~~~~~~~~~~~~~~
    'SessionRequest' was also declared here.

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2316:7 - error TS2451: Cannot redeclare block-scoped variable 'STOPPED'.

2316   let STOPPED : string ;
           ~~~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:109:9
    109         STOPPED = "stopped"
                ~~~~~~~
    'STOPPED' was also declared here.

node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2320:7 - error TS2451: Cannot redeclare block-scoped variable 'isAvailable'.

2320   let isAvailable : boolean ;
           ~~~~~~~~~~~

  node_modules/@types/chrome/chrome-cast/index.d.ts:122:16
    122     export var isAvailable: boolean;
                       ~~~~~~~~~~~
    'isAvailable' was also declared here.

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

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

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

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

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

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

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

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

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

2463   let settings : google.ima.ImaSdkSettings ;
                                 ~~~~~~~~~~~~~~

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

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

node_modules/@types/chrome/chrome-cast/index.d.ts:109:9 - error TS2451: Cannot redeclare block-scoped variable 'STOPPED'.

109         STOPPED = "stopped"
            ~~~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2316:7
    2316   let STOPPED : string ;
               ~~~~~~~
    'STOPPED' was also declared here.

node_modules/@types/chrome/chrome-cast/index.d.ts:122:16 - error TS2451: Cannot redeclare block-scoped variable 'isAvailable'.

122     export var isAvailable: boolean;
                   ~~~~~~~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2320:7
    2320   let isAvailable : boolean ;
               ~~~~~~~~~~~
    'isAvailable' was also declared here.

node_modules/@types/chrome/chrome-cast/index.d.ts:206:18 - error TS2300: Duplicate identifier 'ApiConfig'.

206     export class ApiConfig {
                     ~~~~~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2269:9
    2269   class ApiConfig {
                 ~~~~~~~~~
    'ApiConfig' was also declared here.

node_modules/@types/chrome/chrome-cast/index.d.ts:232:18 - error TS2300: Duplicate identifier 'Error'.

232     export class Error {
                     ~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2276:9
    2276   class Error {
                 ~~~~~
    'Error' was also declared here.

node_modules/@types/chrome/chrome-cast/index.d.ts:280:18 - error TS2300: Duplicate identifier 'SessionRequest'.

280     export class SessionRequest {
                     ~~~~~~~~~~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2309:9
    2309   class SessionRequest {
                 ~~~~~~~~~~~~~~
    'SessionRequest' was also declared here.

node_modules/@types/chrome/chrome-cast/index.d.ts:300:18 - error TS2300: Duplicate identifier 'Session'.

300     export class Session {
                     ~~~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2293:9
    2293   class Session {
                 ~~~~~~~
    'Session' was also declared here.

node_modules/@types/chrome/chrome-cast/index.d.ts:452:18 - error TS2300: Duplicate identifier 'Receiver'.

452     export class Receiver {
                     ~~~~~~~~

  node_modules/shaka-player/dist/shaka-player.compiled.d.ts:2286:9
    2286   class Receiver {
                 ~~~~~~~~
    'Receiver' was also declared here.

RaymondCheng pushed a commit to RaymondCheng/shaka-player that referenced this issue Apr 15, 2021
Partially fixes shaka-project#1030. The issue was created to track a problem using
Shaka from a Typescript project. Inside the issue, a fix was contributed
by @alexandercerutti, although it was acknowledged that the fix was not
a complete fix. Event types are still missing, for instance. Having said
that, it seems the main reason that @alexandercerutti's partial fix
wasn't applied is that @joeyparrish expressed a desire for a small
Typescript sample to be added as an automated test to verify that this
works.

Changes
1) Modified generateTsDefs.py to apply the partial fix by
@alexandercerutti.
2) While testing, I discovered that the Windows build produces a broken
.d.ts, because the bytes on Windows (b'?_?.clutz.') are different
than Linux (b'\xe0\xb2\xa0_\xe0\xb2\xa0.clutz.'). Fixed by adding a
second replace pattern for Windows. Since the two are mutually
exclusive, I just run them serially without condition.
3) While verifying, I noticed some unreplaced clutz namespaces. These
occur because instead of ".clutz." the pattern is ".clutz ". I looked
into a couple, they seem to have been supplied because the Closure
compiler lacks declarations. Added Linux and Windows replacement lines
to assign these to the "shakaExterns" namespace.
4) Added a simple Typescript compilation test. I structured the
tsconfig.json to look like a typical usage case where shaka-player is
imported as a node module. Note that if we set skipLibCheck to false,
this leads to the errors described in Issue shaka-project#3185, eg. error TS2694:
Namespace 'google.ima.dai.api' has no exported member 'StreamRequest'
(also mentioned in Issue shaka-project#1030). NOTE that when this test fails, I
couldn’t figure out how to get the compiler output on-screen, so instead
I provide instructions for re-running the test interactively, which will
produce the error on-screen.

Testing Procedure
1) python build/test.py. Confirm pass.
2) Verify that some of the previously unreplaced clutz namespaces, such
as ಠ_ಠ.clutz.AirPlayEvent, are no longer under the ಠ_ಠ.clutz prefix
and instead are under the shakaExterns namespace, and
that you can compile successfully.
3) Repeat both of the above steps under both Linux and Windows. Confirm
that the produced .d.ts files differ only in the comments, where the
source file path is stated.
@joeyparrish joeyparrish added the priority: P3 Useful but not urgent label May 5, 2021
@joeyparrish joeyparrish modified the milestones: v3.2, v3.3 Jul 7, 2021
@benbro
Copy link

benbro commented Oct 17, 2021

Is there a workaround? We are stuck with shaka-player 3.0.5 in angular 11 project.

@joeyparrish joeyparrish removed their assignment Oct 18, 2021
@philippe-elsass-deltatre
Copy link

philippe-elsass-deltatre commented Nov 4, 2021

Not sure it's about interaction with other libraries. Even the simplest TS project doesn't compile.

// index.ts
import { polyfill, Player } from 'shaka-player';
polyfill.registerAll();
const player = new Player();
player.load('https://dash.akamaized.net/akamai/bbb_30fps/bbb_30fps.mpd');

Compiling:

$ npm install shaka-player typescript
[... installs [email protected], [email protected]]

$ npx tsc --lib dom,es2015 index.ts
index.ts:1:34 - error TS2306: File '/redacted/node_modules/shaka-player/dist/shaka-player.compiled.d.ts' is not a module.

1 import { polyfill, Player } from 'shaka-player';
                                   ~~~~~~~~~~~~~~

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

[...]

Found 10 errors.

Only workaround I found was to delete the .d.ts file...

@dwassing
Copy link

Just wanted to leave a comment here, as I ran into this issue today as well (as mentioned by @philippe-elsass-deltatre above).

I wanted to try out shaka drm capabilities with a simple Angular project (following the official guide), but adjusting for webpack (import * as shaka from 'shaka-player'; etc), but I found myself forced to go with the suggested approach in the guide, adding src/assets/shaka-player.ui.js as a script dependency with declare let shaka: any;, using shaka-player 3.2.1.

Any word on this would be greatly appreciated. If Shaka performs to my expectations then I'm very much interested in migrating to the Shaka Player in my webapp.

@philippe-elsass-deltatre

Honestly for now I have to stay with latest 2.x (2.5.9). It's stable and doesn't get in the way.

@EyMaddis
Copy link

We are also stuck on an old version with no end in sight

@joeyparrish
Copy link
Member

We apologize for the generated Typescript definitions. This is not a Typescript-native project, and the tools we are using to generate the definitions are clearly not able to do everything we need. I will increase the priority of this issue.

In the meantime, you may wish to work around the issue with hand-written definitions to import Shaka Player into your Typescript project. We apologize for the inconvenience.

@joeyparrish joeyparrish self-assigned this Nov 22, 2021
@joeyparrish joeyparrish added priority: P2 Smaller impact or easy workaround and removed priority: P3 Useful but not urgent labels Nov 22, 2021
@EyMaddis
Copy link

EyMaddis commented Jan 27, 2022

Any updates on this? :)
We played around with excluding the wrong type definitions but did struggle quite a bit so we are currently still stuck on the old version... :/

Maybe someone else can provide their workaround?

@elsassph
Copy link
Contributor

Current workaround: a post-install script deleting all the .d.ts of the package 😅

@PabloMoragaMeli
Copy link

Current workaround: a post-install script deleting all the .d.ts of the package 😅

what is the script?

@philippe-elsass-deltatre

@PabloMoragaMeli

rm ./node_modules/shaka-player/dist/*.d.ts

@avelad avelad modified the milestones: v3.3, v4.1 May 4, 2022
@avelad avelad modified the milestones: v4.2, v4.3 Aug 17, 2022
@avelad avelad modified the milestones: v4.3, v4.4 Nov 11, 2022
@Security2431
Copy link

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

@avelad avelad modified the milestones: v4.4, v4.5 Aug 31, 2023
@avelad avelad modified the milestones: v4.5, v4.6 Oct 5, 2023
@siarheipashkevich
Copy link

@Security2431 how do you think is it possible to remove some built-in plugins from compiled code using path-package?

For avoiding https://github.com/shaka-project/shaka-player/blob/main/docs/tutorials/plugins.md#excluding-default-plugins this

@Security2431
Copy link

@siarheipashkevich You can remove 'em from import. However, they'll still up in node_modules. What is the end purpose of it?

@siarheipashkevich
Copy link

I want to decrease the bundle size of my player. Right now shaka used ~500kb.
For example I don't need subtitles in my player and if I remove the plugin for subtitles then the bundle size may be decreased.

@Security2431
Copy link

@siarheipashkevich Ah, I see the point. Well, it's not your case though. path-package will continue store all the deleted lines just in different place

@siarheipashkevich
Copy link

@Security2431 could you please ping me in TG siarheipashkevich for possible solution?

@avelad avelad modified the milestones: v4.6, v5.0 Nov 16, 2023
@avelad avelad modified the milestones: v4.7, v5.0 Dec 4, 2023
@rbertramResi
Copy link

rbertramResi commented Dec 4, 2023

Question/Suggestion on this: Would it be better to prevent .d.ts files from being shipped with the npm package since it breaks typescript projects by default? Maybe they could be provided else where so we could include them and modify them as needed in the project with copy/paste?

@avelad avelad modified the milestones: v4.8, v4.9 Apr 26, 2024
@avelad avelad modified the milestones: v4.9, v4.10 May 30, 2024
@cristian-atehortua
Copy link
Contributor

cristian-atehortua commented Jun 25, 2024

Hi @joeyparrish, I was taking a look and I see that making to only include content of externs/shaka folder on the generation of type definitions allows us to exclude non-Shaka externs from the .d.ts files.

--- a/build/build.py
+++ b/build/build.py
@@ -316,7 +316,7 @@ class Build(object):
       return False
 
     generated_externs = [extern_generator.output]
-    shaka_externs = shakaBuildHelpers.get_all_js_files('externs')
+    shaka_externs = shakaBuildHelpers.get_all_js_files('externs/shaka')
     if self.has_ui():
       shaka_externs += shakaBuildHelpers.get_all_js_files('ui/externs')
     ts_def_generator = compiler.TsDefGenerator(

Do you think it will have another unwanted effect or can I submit a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.