Skip to content

Conversation

@smith558
Copy link
Contributor

@smith558 smith558 commented Feb 21, 2025

This should be considered. The performance gains are not negligible. Additionally, find-my-way and Fastify use it by default, so there's no reason to regress on this for the adapter.

The querystring is also absolutely deprecated with the last feature commit from 2018 (it seems). @kamilmysliwiec
image

Related to #10248.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 38450cb8-a4f4-4360-852f-b9183c3e5386

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 89.292%

Totals Coverage Status
Change from base Build 57005691-848e-40a1-8900-5c194e43249e: 0.0%
Covered Lines: 7146
Relevant Lines: 8003

💛 - Coveralls

@kamilmysliwiec
Copy link
Member

image

https://fastify.dev/docs/latest/Reference/Server/#querystringparser

@smith558
Copy link
Contributor Author

smith558 commented Feb 22, 2025

image https://fastify.dev/docs/latest/Reference/Server/#querystringparser

This was a mistake in the docs and is being fixed. If you look at the source code it's clear they actually use fast-querystring. You may also look at fastify/fastify#5993.

@smith558
Copy link
Contributor Author

This has now been fixed on the Fastify side (fastify/fastify#5993). Let's merge this if you're happy with it. @kamilmysliwiec

@kamilmysliwiec kamilmysliwiec merged commit 71c30d4 into nestjs:master Feb 28, 2025
3 checks passed
@smith558 smith558 deleted the new-fastify-parser branch February 28, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants