gatsby-uc/plugins

bug(gatsby-plugin-fastify): splat redirects not working

Closed this issue ยท 8 comments

Describe the bug

The docs on redirects are not very clear/existing. There is really only the section on Reverse Proxy (which I assume is specific to redirects with statusCode: 200) so I'm not really sure if this is a bug or intended behaviour.

Most of the redirect types from the Gatsby docs do not work. https://support.gatsbyjs.com/hc/en-us/articles/1500003051241-Working-with-Redirects-and-Rewrites

// works, redirects to: /recipes/mouthwatering-lasagna
createRedirect({
  fromPath: `/blog/recipes/mouthwatering-lasagna`,
  toPath: `/recipes/mouthwatering-lasagna`,
}) 
// works, redirects all URLs to /recipes including the exact /blog/recipes/*
createRedirect({
  fromPath: `/blog/recipes/*`,
  toPath: `/recipes`,
})
// doesn't work, redirects all paths after recipes2 to /recipes2/* (with asterisk in URL)
createRedirect({
  fromPath: `/blog/recipes2/*`,
  toPath: `/recipes2/*`,
})
// doesn't work, no redirect at all, just loads /param?id=7
createRedirect({
  fromPath: `/param?id=7`,
  toPath: `/gatsby_file.pdf`
})
// doesn't work, no redirect, loads /param?id=someid (or other param passed)
createRedirect({
  fromPath: `/param?id=:id`,
  toPath: `/param/:id`,
})

To Reproduce

Create the redirects above or clone https://github.com/tsdexter/gatsby-plugin-fastify-redirects-bug

Expected behavior

I would expect splat and param redirects to work

System Info

Please provide information about your site via these means as possible:

System:
OS: Windows 10 10.0.19044
CPU: (16) x64 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
Binaries:
Node: 16.16.0 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.19 - C:\Program Files\nodejs\yarn.CMD
npm: 8.11.0 - C:\Program Files\nodejs\npm.CMD
Languages:
Python: 2.7.8 - /c/Python27/python
Browsers:
Edge: Spartan (44.19041.1266.0), Chromium (106.0.1370.42)
npmPackages:
gatsby: ^4.22.0 => 4.24.4
gatsby-plugin-fastify: ^0.9.4 => 0.9.4
gatsby-plugin-google-analytics: ^4.16.0 => 4.24.0
gatsby-plugin-google-tagmanager: ^4.24.0 => 4.24.0
gatsby-plugin-image: ^2.16.1 => 2.24.0
gatsby-plugin-manifest: ^4.16.0 => 4.24.0
gatsby-plugin-postcss: ^5.16.0 => 5.24.0
gatsby-plugin-react-helmet: ^5.16.0 => 5.24.0
gatsby-plugin-sharp: ^4.16.1 => 4.24.0
gatsby-plugin-sitemap: ^5.24.0 => 5.24.0
gatsby-source-filesystem: ^4.16.0 => 4.24.0
gatsby-source-wordpress: ^6.16.1 => 6.24.0
gatsby-transformer-sharp: ^4.16.0 => 4.24.0
npmGlobalPackages:
gatsby-dev-cli: 4.24.0

For context, so far, this plugin has not tried to maintain compatibility with Gatsby/Gatsby Cloud in specific syntax. For example, I call out in the docs for Gatsby Function routes use the Fastify API and not the Express style Gatsby usually follows. The docs could be more clear that this also applies to the router and routing syntax. Meaning you'd need to use Fastify routing syntax to do these, and not the Gatsby Cloud syntax. Like I mentioned in issue #270 it's possible we could create a compatibility layer for the Gatsby style routing but that doesn't exist currently.

Fastify uses find-my-way as it's router, you can see what routing syntax is supported in it's docs.

Just, looking at the code that implements redirects, I'm thinking this may be on that code partially or completely. Will do some digging

Thanks @moonmeister appreciate all the work you're doing ๐Ÿป๐Ÿป. No issue on the syntax differences, I've already got functions running with the fastify style, in fact, I did a little extra work so they work on Gatsby Cloud and Fastify (we're testing on both and eventually may move to gatsby cloud during renewal)

// /api/some-function.ts
import { GatsbyFunctionRequest, GatsbyFunctionResponse } from "gatsby"
import type { FastifyRequest, FastifyReply } from "fastify"

type Request = GatsbyFunctionRequest | FastifyRequest
type Response = GatsbyFunctionResponse | FastifyReply

function isGatsbyHost(res: Response): res is GatsbyFunctionResponse {
  return (res as GatsbyFunctionResponse).setHeader !== undefined
}

function setHeaders(res: Response): void {
  const gatsybCloudHeaders = [
    { name: `Access-Control-Allow-Headers`, value: `Content-Type` },
    {
      name: `cache-control`,
      value: `public, max-age=120, s-maxage=120 stale-while-revalidate=180`,
    },
  ]
  const fastifyHeaders = {
    "Access-Control-Allow-Headers": `Content-Type`,
    "cache-control": `public, max-age=120, s-maxage=120 stale-while-revalidate=180`,
  }

  if (isGatsbyHost(res)) {
    gatsybCloudHeaders.forEach(header => {
      res.setHeader(header.name, header.value)
    })
  } else {
    res.headers(fastifyHeaders)
  }
}

export default async function handler(
  req: Request,
  res: Response
): Promise<void> {
  // do some function stuff
  res.statusCode = 200
  setHeaders(res)
  res.send(JSON.stringify({data: "hi"}))
}

I will do some testing with Fastify style syntax and see whats working. As for assistance on a fix I probably won't be able to figure out contributing (#270) for a bit as we're working through a backlog on the frontend but I'll see if I can get in some time at home on the mac.

So far I'm only having issues with the query parameters style /test?id=:id path, the standard path parmas /test/:param and /test/* are working fine.

okay, I think I figured out why, the query params aren't working...I know what needs to change. Just need to see if there's a library or I need to write this code myself.

So far I'm only having issues with the query parameters style /test?id=:id path, the standard path parmas /test/:param and /test/* are working fine.

You got this working in the current version or you've made some changes to get it working?

Working on changes.

@moonmeister just looking through the PR, nice work on the changes... I see you must've also ran into the server failing to start from duplicate redirects... I also used a Set to get around that when registering them from a JSON file managed by marketing with many duplicates ๐Ÿ‘

        const size = redirectsSet.size
        redirectsSet.add(trimmedPath)
        if (redirectsSet.size === size) {
          reporter.warn(`skipping duplicate redirect for: ${trimmedPath}`)
        } else {
          reporter.success(
            `creating redirect: ${newFromPath} -> ${redirect.toPath}`
          )
          createRedirect({
            ...redirect,
            fromPath: newFromPath,
          })
        }