typings / registry

The registry of type definitions for TypeScript
238 stars 176 forks source link

Update with PR requested by @unional #926

Closed shawnlauzon closed 7 years ago

shawnlauzon commented 7 years ago

Typings URL: https://github.com/shawnlauzon/typed-soundcloud

Questions (for new typings):

Change Summary (for existing typings):

From @unional's PR:

Refactored the code. Also fixed the jspm test. I need to make some changes to generator-typings

felixfbecker commented 7 years ago

Why not do the typings ES6-style?

import { Promise } from 'es6-promise';
export function initialize(options: InitializeOptions);
export function connect(options?: ConnectOptions): Promise<any>;

export function get(path: string, params?: any): Promise<any>;
export function post(path: string, params?: any): Promise<any>;
export function put(path: string, params?: any): Promise<any>;
export function delete(path: string): Promise<any>;

export function upload(options: any): Promise<any>;
export function resolve(url: string): Promise<any>;
export function oEmbed(url: string, params?: any): Promise<any>;

export function stream(trackPath: string, secretToken?: string): Promise<Player>;

export interface InitializeOptions {
    client_id: string;
    redirect_uri?: string;
    oauth_token?: string;
  }

export interface ConnectOptions {
    client_id: string;
    redirect_uri: string;
    scope?: string;
  }

 export interface Player {
    play();
    pause();
    seek(time: number);
    currentTime(): number;
    setVolume(volume: number);
    getVolume(): number;
    on(event: string, handler: any);
  }
felixfbecker commented 7 years ago

Also, Player.play() and others are missing a return type annotation - this will cause compilation errors with noImplicitAny. You should add that option to tsconfig.json

unional commented 7 years ago

This was done quite a while ago. Yes, Player.play() and others can add the return type. As for ES6 vs CommonJS, I think either is fine, but my general suggestion is to keep the typings the same as what the source is to avoid any interop confusion.

shawnlauzon commented 7 years ago

Sounds good. I'll add those return types and add noImplicitAny to tsconfig.json. As @unional suggests, I'll leave the others as they are.