Skip to content

Conversation

@felixputera
Copy link
Contributor

@felixputera felixputera commented Mar 27, 2020

Hello 👋

First of all, thanks @mcollina, @delvedor & all contributors for the awesome fastify ecosystem. I had just recently dived into it & I'm very impressed.

Moving on to the issue on hand, I found out that the TS typing provided for PluginOptions (used in fastify.register(ws, opts \*here*\) is incorrect: the typings requires both handler and options to be passed in. I looked into the docs & code to verify that it has defaults specified.

I fixed this in this PR, and had added tests to cover this use-case. Additionally, I did minor clean-up on the declaration file.

I think this addresses #47 too.

Checklist

  • run npm run test and npm run benchmark (I couldn't find npm run bench nor npm run benchmark, should the PR template be updated?)
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

addtionally, tests were added to cover this change
remove unused imports & clean up dangling whitespaces
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
Copy link
Member

Thanks, I'm glad that you like it.

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

💪🏻

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 merged commit 30c3ae3 into fastify:master Mar 28, 2020
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.

3 participants