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

fix for typescript http2/https warning #115

Merged
merged 5 commits into from
Nov 29, 2019
Merged

fix for typescript http2/https warning #115

merged 5 commits into from
Nov 29, 2019

Conversation

ziimakc
Copy link
Contributor

@ziimakc ziimakc commented Nov 20, 2019

Fix for #114

I'm not an expert in typescript and declaration files, but i was able to adapt fastify-static declaration file in style of fastify-cookie declaration file (that gives no typescript warnings).

Now it should compile without problems for http2 and htpps too.

Checked on my own server with http, http2 and https settings for server.

@ziimakc
Copy link
Contributor Author

ziimakc commented Nov 20, 2019

Hm, change of index.d.ts can break somehow test?

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.

Can you cover this in the types tests?

@ziimakc
Copy link
Contributor Author

ziimakc commented Nov 21, 2019

@mcollina changed types/index.ts in fastify-cookies style, if it's what you mean.

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

index.d.ts Show resolved Hide resolved
Copy link
Member

@Ethan-Arrowood Ethan-Arrowood left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you good work 👍

@Eomm
Copy link
Member

Eomm commented Nov 22, 2019

@Ethan-Arrowood is this a patch for TS's users?

@Ethan-Arrowood
Copy link
Member

Yes this is a patch fix 👍

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.

4 participants