Update v4 migration guide to use querystring instead of query

[multiwebinc]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.5

Plugin version

No response

Node.js version

16

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Ubuntu 22.04

Description

In Fastify v3, you could use either query or querystring for route validations, but for fastify v4, query has been removed. This is not mentioned in the Migration guide. I upgraded from 3.14.1 and I don't remember ever seeing any deprecation notices about this, and it is also not mentioned as being deprecated on the 3.29.x docs.

Steps to Reproduce

n/a

Expected Behavior

No response

[multiwebinc]

Wait, on second thought, I was confused about two parts of the docs. Maybe it is still supposed to work with query because it is mentioned here, but this is actually a bug in the code. For example, this creates the error FastifyError: Schema with 'querystring' already present!:

async function routes(fastify) {
  fastify.get(
    '/api/properties/:slug/availability',
    {
      schema: {
        query: Yup.object().shape({
        }),
      },
      validatorCompiler: () => true,
      handler: availabilityController.getAvailability,
    }
  );
}

Changing query to querystring works as expected.

[climba03003]

query should works as usual, maybe somewhere inside your code mess it up.
For example, onRoute hooks modify the schema.

I do not use yup, but it should not make any different.

import Fastify from 'fastify'

const fastify = Fastify()

fastify.get('/', {
  schema: {
    query: { type: 'object', properties: { hello: { const: 'world' } } }
  }
}, (request) => request.query)

const response = await fastify.inject('/?hello=world')
console.log(response.body)

[multiwebinc]

@climba03003 The problem arises when I use Yup. I just did a fresh install of Fastify (npm init fastify then npm i yup) and get the same error. Here is my routes/root.js:

'use strict'

const Yup = require('yup');

module.exports = async function (fastify, opts) {
  fastify.get('/', {
    schema: {
      query: Yup.object(),
    },
    validatorCompiler: () => () => true,
  }, (request) => request.query)
}

But if I swap out query for what you have, or rename it as querystring using Yup, then it works.

Would this be a bug with Fastify or with Yup?

[climba03003]

Would this be a bug with Fastify or with Yup?

Yes, I can confirm it is the problem with custom class object.

The problem caused by we mutated the schema but return early when schema is not a raw object.

fastify/lib/schemas.js

Lines 63 to 67 in 6f6f7aa

	for (const key of ['headers', 'querystring', 'params', 'body']) {
	if (typeof routeSchemas[key] === 'object' && Object.getPrototypeOf(routeSchemas[key]) !== Object.prototype) {
	return routeSchemas
	}
	}

Then, when it comes to auto HEAD route, the schema cloned do not contain kSchemaVisited and leads to failure.

Would you like to send a PR to address the issue?

[multiwebinc]

@climba03003 I would make a PR, but I am really totally unfamiliar with the codebase. Even with your description I have no idea what would need to be changed.

[Uzlopak]

I think @climba03003 meant that you provide a PR not the other way round.

[climba03003]

@climba03003 I would make a PR, but I am really totally unfamiliar with the codebase. Even with your description I have no idea what would need to be changed.

Add routeSchemas[kSchemaVisited] = true right before L65 should do the trick.
Remember to add an unit test to avoid regression.

I think @climba03003 meant that you provide a PR not the other way round.

Oh, I click the wrong button.
I plan to reply, but clicked edit.

Sorry for the confusion.