Skip to content

Conversation

@airhorns
Copy link
Member

This corrects an issue introduced in #64 where upon adding fastify-websocket to a project all route handlers were (accidentally) assumed to be websocket handlers getting the different (and decidedly less useful) types. My bad! This corrects the issue by using a type-land overload of the RouteShorthand function definition to change the type of the handler only if the handler is in fact { websocket: true }.

Also adds tests, tsd is handy!

Checklist

  • run npm run test and npm run benchmark
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message and code follows Code of conduct

@airhorns airhorns force-pushed the different-types-for-websocket-handlers-only branch from c075f38 to 42c56eb Compare July 16, 2020 04:43
@airhorns
Copy link
Member Author

Please hold off on merging this I am fighting with another issue that might be relevant ... TBD

@airhorns airhorns changed the title Ensure websocket handler types are only applied to websocket handlers WIP Ensure websocket handler types are only applied to websocket handlers Jul 16, 2020
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

This corrects an issue introduced in fastify#64 where upon adding `fastify-websocket` to a project all route handlers were (accidentally) assumed to be websocket handlers getting the different (and decidedly less useful) types. My bad! This corrects the issue by using a type-land overload of the RouteShorthand function definition to change the type of the handler only if the handler is in fact `{ websocket: true }`.

Also adds tests, `tsd` is handy!
@airhorns airhorns force-pushed the different-types-for-websocket-handlers-only branch from 42c56eb to b0589a6 Compare July 16, 2020 15:09
@airhorns airhorns changed the title WIP Ensure websocket handler types are only applied to websocket handlers Ensure websocket handler types are only applied to websocket handlers Jul 16, 2020
@airhorns
Copy link
Member Author

Ok -- this is good to go. Developing these types is real tricky because all the types across all the transitive dependencies have to agree exactly. My issue was that transitive dependencies in my project were including older types and when npm linking it gets even crazier. yalc helped a lot with this and I feel good that this is working now!

@mcollina
Copy link
Member

Nice job! yalc seems very useful, I did not know it!

@mcollina mcollina merged commit 2b5cef7 into fastify:master Jul 17, 2020
@airhorns airhorns deleted the different-types-for-websocket-handlers-only branch July 17, 2020 13:06
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