light-my-request icon indicating copy to clipboard operation
light-my-request copied to clipboard

inject doesn't like the new HTTP QUERY Method

Open andokai opened this issue 1 year ago • 2 comments

Prerequisites

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

Fastify version

5.0.0

Plugin version

No response

Node.js version

22.10

Operating system

macOS

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

15.0.1

Description

With the PR added to find-my-way https://github.com/delvedor/find-my-way/pull/380, Fastify can now support the QUERY method. However, testing the route using inject throws an error as the method is not in the hard coded list here.

https://github.com/fastify/light-my-request/blob/11f2447974482a1af5b75f894d97af5c9a39794c/lib/config-validator.js#L862

If I add QUERY to that line then the tests succeed. I'm happy to submit a PR with this change but wanted to confirm that there wasn't anything else that I need to bear in mind.

For example there's an array of what appear to be the most common HTTP methods here.

https://github.com/fastify/light-my-request/blob/11f2447974482a1af5b75f894d97af5c9a39794c/index.js#L89

Should I be adding 'query' to that array?

Link to code that reproduces the bug

No response

Expected Behavior

No response

andokai avatar Oct 25 '24 10:10 andokai

Should I be adding 'query' to that array?

No, that code does not affect light-my-request/lib/config-validator.js, it generates the req.query() method shortcut (that we are missing and still it is an issue)

The real issue is here:

https://github.com/fastify/light-my-request/blob/11f2447974482a1af5b75f894d97af5c9a39794c/build/build-validation.js#L61

We should:

  1. force the light-my-request http method list
  2. AND/OR force the script to throw if a maintainer build the config-validator.js file using nodejs != 22

Eomm avatar Oct 25 '24 12:10 Eomm

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

mcollina avatar Oct 25 '24 13:10 mcollina