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

Remove override of the default route #238

Merged
merged 1 commit into from
Jan 8, 2023
Merged

Remove override of the default route #238

merged 1 commit into from
Jan 8, 2023

Conversation

mcollina
Copy link
Member

@mcollina mcollina commented Jan 8, 2023

Signed-off-by: Matteo Collina hello@matteocollina.com

Just so you know, the change in the test is unrelated. That test checked if we trying to connect to a known route, and /echo/ was not registered, only /echo was.

Fixes #236.
Fixes platformatic/platformatic#575.

Checklist

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@airhorns
Copy link
Member

airhorns commented Jan 8, 2023

Yeah, looking into it, I think that code was an attempt to keep support for a global websocket connection handler that this library used to support. Once this library started re-using fastify's router, that stopped being a thing though -- a route has to be matched before it can be upgraded. If users want one handler to process all incoming websocket connections, they should use a fastify.get("/*", {websocket: true}, ...), so I think this can be removed.

@mcollina mcollina merged commit b81515c into master Jan 8, 2023
@Uzlopak Uzlopak deleted the fix-236 branch January 8, 2023 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants