This repository has been archived by the owner. It is now read-only.

Incorrect TypeScript types for S3 customHeaders #1990

Open
bradleyayers opened this Issue Mar 28, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@bradleyayers
Copy link
Contributor

bradleyayers commented Mar 28, 2018

Type of issue

  • Bug report
  • Feature request

Uploader type

  • Traditional
  • S3
  • Azure

Current:

/**
 * type for S3's customHeaders function
 */
export interface S3CustomHeaderFunction {
    (id: number): void;
}

[snip]

/**
 * Additional headers sent along with each signature request.
 *
 * If you declare a function as the value, the associated file's ID will be passed to your function when it is invoked
 *
 * @default `{}`
 */
customHeaders?: any | S3CustomHeaderFunction;

The type should really be something like this:

customHeaders?: 
  | { [headerName: string]: string }
  | ((fileId: number) => { [headerName: string]: string });
@SinghSukhdeep

This comment has been minimized.

Copy link
Member

SinghSukhdeep commented Mar 30, 2018

Is this working for you?

customHeaders?: 
  | { [headerName: string]: string }
  | (fileId: number) => { [headerName: string]: string };

Isn't there an extra | at the beginning or I'm not reading it right

@SinghSukhdeep

This comment has been minimized.

Copy link
Member

SinghSukhdeep commented Mar 30, 2018

And I'm seeing some issues here

fu-type-error

@bradleyayers

This comment has been minimized.

Copy link
Contributor

bradleyayers commented Mar 30, 2018

Isn't there an extra | at the beginning or I'm not reading it right

TypeScript has supported leading | for a while, see Microsoft/TypeScript#12071. It was purely a stylistic choice from me to try and visually separate the two items of the union.

I'm going to wait for #1989 before raising a PR for this.

And I'm seeing some issues here

Thanks I was missing some parenthesis around the function, I've updated the snippet.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.