[Backport 4.x] refactor: change reply.redirect() signature (#5483) (#…

…5484)

* feat: change `reply.redirect()` signature

* feat: change `reply.redirect()` signature

* docs

* docs

* update message

* fix deprecation

* update message

docs/Reference/Reply.md

@@ -17,7 +17,7 @@
   - [.trailer(key, function)](#trailerkey-function)
   - [.hasTrailer(key)](#hastrailerkey)
   - [.removeTrailer(key)](#removetrailerkey)
-  - [.redirect([code ,] dest)](#redirectcode--dest)
+  - [.redirect(dest, [code ,])](#redirectdest--code)
   - [.callNotFound()](#callnotfound)
   - [.getResponseTime()](#getresponsetime)
   - [.type(contentType)](#typecontenttype)
@@ -62,8 +62,8 @@ since the request was received by Fastify.
 - `.hasTrailer(key)` - Determine if a trailer has been set.
 - `.removeTrailer(key)` - Remove the value of a previously set trailer.
 - `.type(value)` - Sets the header `Content-Type`.
-- `.redirect([code,] dest)` - Redirect to the specified URL, the status code is
-  optional (default to `302`).
+- `.redirect(dest, [code,])` - Redirect to the specified URL, the status code is
+  optional (defaults to `302`).
 - `.callNotFound()` - Invokes the custom not found handler.
 - `.serialize(payload)` - Serializes the specified payload using the default
   JSON serializer or using the custom serializer (if one is set) and returns the
@@ -299,7 +299,7 @@ reply.getTrailer('server-timing') // undefined
 ```
 
 
-### .redirect([code ,] dest)
+### .redirect(dest, [code ,])
 <a id="redirect"></a>
 
 Redirects a request to the specified URL, the status code is optional, default
@@ -320,7 +320,7 @@ reply.redirect('/home')
 Example (no `reply.code()` call) sets status code to `303` and redirects to
 `/home`
 ```js
-reply.redirect(303, '/home')
+reply.redirect('/home', 303)
 ```
 
 Example (`reply.code()` call) sets status code to `303` and redirects to `/home`
@@ -330,7 +330,7 @@ reply.code(303).redirect('/home')
 
 Example (`reply.code()` call) sets status code to `302` and redirects to `/home`
 ```js
-reply.code(303).redirect(302, '/home')
+reply.code(303).redirect('/home', 302)
 ```
 
 ### .callNotFound()

docs/Reference/Warnings.md

@@ -24,6 +24,7 @@
     - [FSTDEP018](#FSTDEP018)
     - [FSTDEP019](#FSTDEP019)
     - [FSTDEP020](#FSTDEP020)
+    - [FSTDEP021](#FSTDEP021)
 
 
 ## Warnings
@@ -90,3 +91,4 @@ Deprecation codes are further supported by the Node.js CLI options:
 | <a id="FSTDEP018">FSTDEP018</a> | You are accessing the deprecated `request.routerMethod` property. | Use `request.routeOptions.method`. | [#4470](https://github.com/fastify/fastify/pull/4470) |
 | <a id="FSTDEP019">FSTDEP019</a> | You are accessing the deprecated `reply.context` property. | Use `reply.routeOptions.config` or `reply.routeOptions.schema`. | [#5032](https://github.com/fastify/fastify/pull/5032) [#5084](https://github.com/fastify/fastify/pull/5084) |
 | <a id="FSTDEP020">FSTDEP020</a> | You are using the deprecated `reply.getReponseTime()` method. | Use the `reply.elapsedTime` property instead. | [#5263](https://github.com/fastify/fastify/pull/5263) |
+| <a id="FSTDEP021">FSTDEP021</a> | The `reply.redirect()` method has a new signature: `reply.redirect(url: string, code?: number)`. It will be enforced in `fastify@v5`'. | [#5483](https://github.com/fastify/fastify/pull/5483) |

lib/reply.js

@@ -55,7 +55,7 @@ const {
   FST_ERR_MISSING_SERIALIZATION_FN,
   FST_ERR_MISSING_CONTENTTYPE_SERIALIZATION_FN
 } = require('./errors')
-const { FSTDEP010, FSTDEP013, FSTDEP019, FSTDEP020 } = require('./warnings')
+const { FSTDEP010, FSTDEP013, FSTDEP019, FSTDEP020, FSTDEP021 } = require('./warnings')
 
 const toString = Object.prototype.toString
 
@@ -457,9 +457,15 @@ Reply.prototype.type = function (type) {
   return this
 }
 
-Reply.prototype.redirect = function (code, url) {
-  if (typeof code === 'string') {
-    url = code
+Reply.prototype.redirect = function (url, code) {
+  if (typeof url === 'number') {
+    FSTDEP021()
+    const temp = code
+    code = url
+    url = temp
+  }
+
+  if (!code) {
     code = this[kReplyHasStatusCode] ? this.raw.statusCode : 302
   }
 

lib/warnings.js

@@ -82,6 +82,11 @@ const FSTDEP020 = createDeprecation({
   message: 'You are using the deprecated "reply.getResponseTime()" method. Use the "reply.elapsedTime" property instead. Method "reply.getResponseTime()" will be removed in `fastify@5`.'
 })
 
+const FSTDEP021 = createDeprecation({
+  code: 'FSTDEP021',
+  message: 'The `reply.redirect()` method has a new signature: `reply.redirect(url: string, code?: number)`. It will be enforced in `fastify@v5`'
+})
+
 const FSTWRN001 = createWarning({
   name: 'FastifyWarning',
   code: 'FSTWRN001',
@@ -113,6 +118,7 @@ module.exports = {
   FSTDEP018,
   FSTDEP019,
   FSTDEP020,
+  FSTDEP021,
   FSTWRN001,
   FSTWRN002
 }

test/internals/reply.test.js

@@ -19,7 +19,7 @@ const {
 } = require('../../lib/symbols')
 const fs = require('node:fs')
 const path = require('node:path')
-const { FSTDEP010, FSTDEP019, FSTDEP020 } = require('../../lib/warnings')
+const { FSTDEP010, FSTDEP019, FSTDEP020, FSTDEP021 } = require('../../lib/warnings')
 
 const agent = new http.Agent({ keepAlive: false })
 
@@ -250,15 +250,15 @@ test('within an instance', t => {
   })
 
   fastify.get('/redirect-code', function (req, reply) {
-    reply.redirect(301, '/')
+    reply.redirect('/', 301)
   })
 
   fastify.get('/redirect-code-before-call', function (req, reply) {
     reply.code(307).redirect('/')
   })
 
   fastify.get('/redirect-code-before-call-overwrite', function (req, reply) {
-    reply.code(307).redirect(302, '/')
+    reply.code(307).redirect('/', 302)
   })
 
   fastify.get('/custom-serializer', function (req, reply) {
@@ -2094,6 +2094,36 @@ test('redirect to an invalid URL should not crash the server', async t => {
   await fastify.close()
 })
 
+test('redirect with deprecated signature should warn', t => {
+  t.plan(4)
+
+  process.removeAllListeners('warning')
+  process.on('warning', onWarning)
+  function onWarning (warning) {
+    t.equal(warning.name, 'DeprecationWarning')
+    t.equal(warning.code, FSTDEP021.code)
+  }
+
+  const fastify = Fastify()
+
+  fastify.get('/', (req, reply) => {
+    reply.redirect(302, '/new')
+  })
+
+  fastify.get('/new', (req, reply) => {
+    reply.send('new')
+  })
+
+  fastify.inject({ method: 'GET', url: '/' }, (err, res) => {
+    t.error(err)
+    t.pass()
+
+    process.removeListener('warning', onWarning)
+  })
+
+  FSTDEP021.emitted = false
+})
+
 test('invalid response headers should not crash the server', async t => {
   const fastify = Fastify()
   fastify.route({

test/types/reply.test-d.ts

@@ -29,7 +29,7 @@ const getHandler: RouteHandlerMethod = function (_request, reply) {
   expectAssignable<() => { [key: string]: number | string | string[] | undefined }>(reply.getHeaders)
   expectAssignable<(key: string) => FastifyReply>(reply.removeHeader)
   expectAssignable<(key: string) => boolean>(reply.hasHeader)
-  expectType<{(statusCode: number, url: string): FastifyReply; (url: string): FastifyReply }>(reply.redirect)
+  expectType<{(statusCode: number, url: string): FastifyReply;(url: string, statusCode?: number): FastifyReply;}>(reply.redirect)
   expectType<() => FastifyReply>(reply.hijack)
   expectType<() => void>(reply.callNotFound)
   // Test reply.getResponseTime() deprecation

types/reply.d.ts

@@ -57,9 +57,11 @@ export interface FastifyReply<
   getHeaders(): Record<HttpHeader, number | string | string[] | undefined>;
   removeHeader(key: HttpHeader): FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>;
   hasHeader(key: HttpHeader): boolean;
-  // Note: should consider refactoring the argument order for redirect. statusCode is optional so it should be after the required url param
+  /**
+   * @deprecated The `reply.redirect()` method has a new signature: `reply.reply.redirect(url: string, code?: number)`. It will be enforced in `fastify@v5`'.
+   */
   redirect(statusCode: number, url: string): FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>;
-  redirect(url: string): FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>;
+  redirect(url: string, statusCode?: number): FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>;
   hijack(): FastifyReply<RawServer, RawRequest, RawReply, RouteGeneric, ContextConfig, SchemaCompiler, TypeProvider>;
   callNotFound(): void;
   /**