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

Type for options for POST request not exported #63

Closed
jdforsythe opened this issue Feb 16, 2021 · 2 comments · Fixed by #65
Closed

Type for options for POST request not exported #63

jdforsythe opened this issue Feb 16, 2021 · 2 comments · Fixed by #65

Comments

@jdforsythe
Copy link
Contributor

jdforsythe commented Feb 16, 2021

The type used for POST requests is IWithData<phin.IOptions> but the IWithData is not exported in types.d.ts so there's no way to construct a typed options object without making a custom type.

Also, there's a note in the types about using the generics to make JSON/Form posting mutually exclusive, but wouldn't this work just as well?

export interface IOptions extends IOptionsBase {
 parse?: 'none'
}

interface WithData {
  data: object;
}

interface WithForm {
  form: {
    [index: string]: string;
  }
}

export type IOptionsWithData = IOptions & WithData;
export type IOptionsWithForm = IOptions & WithForm;
// etc
@ethan7g
Copy link
Owner

ethan7g commented Feb 26, 2021

Thanks for pointing that out. A PR would be appreciated!

jdforsythe added a commit to jdforsythe/phin that referenced this issue Mar 2, 2021
Fixes ethan7g#63

Exports IWithData and IWithForm types
Explicitly makes generic extend IOptionsBase
Changes IWithData's data property to be string, Buffer, or object (allows strings, Buffers, objects, or arrays)

No behavior is affected so no new tests were written
@jdforsythe
Copy link
Contributor Author

@ethanent PR submitted #65

I kept the generics but explicitly made the T extend IOptionsBase and updated the data property of IWithData to be compatible with what Centra accepts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants