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

Fix Event id attribute parsing #4614

Merged
merged 3 commits into from
Nov 6, 2024

Conversation

bbert
Copy link
Contributor

@bbert bbert commented Nov 5, 2024

Event's id attribute should be as xs:unsignedInt

@bbert bbert added the Bug label Nov 5, 2024
@dsilhavy
Copy link
Collaborator

dsilhavy commented Nov 6, 2024

@bbert Thanks for the fix, can you please check the index.d.ts for the errors reported by tsc:

Error: index.d.ts(2012,22): error TS2430: Interface 'OfflineRecordEvent' incorrectly extends interface 'Event'.
  Types of property 'id' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: index.d.ts(2129,22): error TS2430: Interface 'CueEnterEvent' incorrectly extends interface 'Event'.
  Types of property 'id' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: index.d.ts(213[7](https://github.com/Dash-Industry-Forum/dash.js/actions/runs/11688662792/job/32549627513?pr=4614#step:8:8),22): error TS2430: Interface 'CueExitEvent' incorrectly extends interface 'Event'.
  Types of property 'id' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: Process completed with exit code 2.

@dsilhavy dsilhavy added this to the 5.0.0 milestone Nov 6, 2024
@bbert
Copy link
Contributor Author

bbert commented Nov 6, 2024

@bbert Thanks for the fix, can you please check the index.d.ts for the errors reported by tsc:

Error: index.d.ts(2012,22): error TS2430: Interface 'OfflineRecordEvent' incorrectly extends interface 'Event'.
  Types of property 'id' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: index.d.ts(2129,22): error TS2430: Interface 'CueEnterEvent' incorrectly extends interface 'Event'.
  Types of property 'id' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: index.d.ts(213[7](https://github.com/Dash-Industry-Forum/dash.js/actions/runs/11688662792/job/32549627513?pr=4614#step:8:8),22): error TS2430: Interface 'CueExitEvent' incorrectly extends interface 'Event'.
  Types of property 'id' are incompatible.
    Type 'string' is not assignable to type 'number'.
Error: Process completed with exit code 2.

In fact there is an issue in typings files with Event type, since there is a conflict between the Event type related to EventStream and the base Event type use for all player events.

Maybe we should define another type for player events:

interface MediaPlayerEvent {
  type: string
}

and then redefine all player event types:

export interface AstInFutureEvent extends MediaPlayerEvent {
  type: MediaPlayerEvents['AST_IN_FUTURE'];
  delay: number;
}
...

@dsilhavy
Copy link
Collaborator

dsilhavy commented Nov 6, 2024

AstInFutureEvent

I agree, we need to distinguish between EventStream.Event and MediaPlayerEvent. I think your suggestion is good, can you address this in this PR as well?

@bbert
Copy link
Contributor Author

bbert commented Nov 6, 2024

AstInFutureEvent

I agree, we need to distinguish between EventStream.Event and MediaPlayerEvent. I think your suggestion is good, can you address this in this PR as well?

Done

@dsilhavy dsilhavy merged commit b1fab66 into Dash-Industry-Forum:development Nov 6, 2024
2 checks passed
@bbert bbert deleted the fix/event-id branch November 6, 2024 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

2 participants