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

Typescript definitions for all exports #953

Closed
vai0 opened this issue Jul 1, 2020 · 6 comments
Closed

Typescript definitions for all exports #953

vai0 opened this issue Jul 1, 2020 · 6 comments

Comments

@vai0
Copy link

vai0 commented Jul 1, 2020

Current Behavior

Typescript definitions for the individual players (react-player/wistia, react-player/youtube, etc...) are missing.

e.g.,

import WistiaPlayer from "react-player/wistia";
...

// JSX element type 'ReactPlayerWistia' does not have any construct or call signatures.ts(2604)
<WistiaPlayer ... /> 

As a result, I need to either

  1. explicitly add my own type declarations, and paint WistiaPlayer with the broad typedefs of ReactPlayer...
// react-player/index.d.ts
declare module "react-player/wistia" {
  import ReactPlayer from "react-player";
  export default ReactPlayer;
}

which isn't exactly great because now props such as onBuffered, onBufferEnded, onEnablePIP, etc... that WistiaPlayer really doesn't support noop and creates a confusing developer experience,

OR

  1. again explicitly add my own type declarations, but completely type out the WistiaPlayer export + props that actually work and don't noop.
// react-player/index.d.ts
declare module "react-player/wistia" {
    import * as React from "react";
    import ReactPlayer from "react-player";

    type WistiaConfig = {
        wistia: {
            // https://wistia.com/support/developers/embed-options#options
            options?: {
                ...
            };
            playerId?: string;
        };
    };

    export type ReactPlayerWistiaProps = {
        url?: string | string[];
        playing?: boolean;
        loop?: boolean;
        controls?: boolean;
        volume?: number;
        muted?: boolean;
        playbackRate?: number;
        width?: string | number;
        height?: string | number;
        style?: React.CSSProperties;
        progressInterval?: number;
        light?: boolean | string;
        wrapper?: keyof JSX.IntrinsicElements;
        config?: WistiaConfig;
        className?: string;
        onReady?(player: ReactPlayer): void;
        onStart?(): void;
        onPlay?(): void;
        onPause?(): void;
        onEnded?(): void;
        onError?(
            error: any,
            data?: any,
            hlsInstance?: any,
            hlsGlobal?: any
        ): void;
        onDuration?(duration: number): void;
        onSeek?(seconds: number): void;
        onProgress?(state: { played: number; playedSeconds: number }): void;
    };
    export default class ReactPlayerWistiaPlayer extends React.Component<
        WistiaPlayerProps,
        any
    > {
        static canPlay(url: string): boolean;
        static canEnablePIP(url: string): boolean;
        static addCustomPlayer(player: ReactPlayer): void;
        static removeCustomPlayers(): void;
        seekTo(amount: number, type?: "seconds" | "fraction"): void;
        getCurrentTime(): number;
        getDuration(): number;
        getInternalPlayer(key?: string): Object;
        showPreview(): void;
    }
}

Expected Behavior

Expect that if I use any of the other exports (react-player/wistia, react-player/youtube, etc...) they're automatically typed and I don't need to create my own typings for them.

@cookpete
Copy link
Owner

cookpete commented Jul 3, 2020

I don’t use Typescript, but I welcome any PRs that add definitions to the codebase in a clean way.

@cookpete
Copy link
Owner

I'm going to close this off as there seems to be no value in keeping it open. Like I said before, any PRs adding better TS support will be welcome.

@JuanM04
Copy link
Contributor

JuanM04 commented Dec 4, 2020

Hello there, I'm working on a PR that fixes this. I have one question:
When I import the react-player/wistia, should the config attribute equal to { wistia: { ... } } or { ... }?

@cookpete
Copy link
Owner

cookpete commented Dec 8, 2020

Hey @JuanM04. Awesome work on the PR. I'm just looking through it now.

When I import the react-player/wistia, should the config attribute equal to { wistia: { ... } } or { ... }?

As of v2.0.0, both are supported due to this crazy merging:

getConfig = memoize((url, key) => {
const { config } = this.props
return merge.all([
defaultProps.config,
defaultProps.config[key] || {},
config,
config[key] || {}
])
})

@isometriq
Copy link

types for custom players would be awesome too

@isometriq
Copy link

isometriq commented Dec 9, 2020

interface ReactPlayerCustomPlayer {
    //static canPlay?():boolean,
    //static canEnablePIP?():boolean,
    //static loopOnEnded?:boolean,
    //static forceLoad?:boolean,
    load(url:string, isReady:boolean|undefined),
    play():void,
    pause():void,
    stop():void,
    seekTo(amount:number):void,
    mute():void,
    unmute():void,
    setVolume(fraction:number):void,
    setPlaybackRate?(rate:number):void,
    setLoop?(loop:boolean):void,
    getDuration():number|null,
    getCurrentTime():number|null,
    getSecondsLoaded():number|null,
    enablePIP?():void,
    disablePIP?():void,
}

cookpete pushed a commit that referenced this issue Jan 20, 2021
Webmaster1116 added a commit to Webmaster1116/video-player that referenced this issue May 20, 2021
webmiraclepro added a commit to webmiraclepro/video-player that referenced this issue Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants