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: improve types and allow augmentation #8545

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

barlock
Copy link

@barlock barlock commented Jan 4, 2024

Description

Fix a few types in player and plugin and allow typescript module augmentation so plugins can modify the players type as expected.

Specific Changes proposed

For type fixes, I used similar patterns found elsewhere in the codebase, just applied following the same pattern. For module augmentation, this pattern should be used:

declare module 'video.js/dist/types/player' {
    // videojs-contrib-quality-levels
    qualityLevels: {
      VERSION: string;
      (): QualityLevelList;
    };
  }
}

Ideally one wouldn't need to reach into dist but I'm unable to do that without created a named export of Player on video.js which I didn't think was wanted. Happy to make the change though. JSDoc seems to prevent me from only exporting the type of Player without exporting the whole class as well.

Requirements Checklist

Copy link

welcome bot commented Jan 4, 2024

💖 Thanks for opening this pull request! 💖

Things that will help get your PR across the finish line:

  • Run npm run lint -- --errors locally to catch formatting errors earlier.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

Copy link

@uzbeki uzbeki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that looks like a decent addition to its types.

@mister-ben mister-ben added the ts TypeScript label Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ts TypeScript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants