Skip to content

Conversation

@Ethan-Arrowood
Copy link
Member

Spent some time trying to complete this tonight. Please review as accurately as possible. All of my types are from reading the code. I based some off of the JSON Schema object but also included anything written in the documentation too. There were a couple any types I'd like to see if we can make more specific, as well as some general questions I have:

  • Is the port property on the InjectOptions.url type of string | number?
  • What is query? Could this be typed as string?
  • What types can payload be? I've implemented it as a generic for now but I think we can scope this to an union type list: string | object | Buffer | NodeJS.ReadableStream (this is actually copied from the current fastify.d.ts types HTTPInjectOptions so I believe it should be safe to implement here as well). Depending on what happens here I'll most likely be removing the generic.
  • Am I correct that Response._lightMyRequest.payloadChunks is of type Array<Buffer>?

Not ready to be merged yet as I'm still playing with tsd to test the new types. It is a young module with a tiny API so it might not be quite ready yet, but I'd like to revisit it with a clear head tomorrow.

Finally, I need to experiment with the declaration file exports, namespace, and some of the internal TSC stuff I used (such as interface) to make sure it is correct.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina mcollina requested a review from delvedor March 26, 2019 12:04
@Ethan-Arrowood
Copy link
Member Author

Pinging @delvedor for review in case the notification was missed

@mcollina mcollina merged commit f9b2a96 into fastify:master Apr 4, 2019
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

Successfully merging this pull request may close these issues.

2 participants