withastro / roadmap

Ideas, suggestions, and formal RFC proposals for the Astro project.
290 stars 29 forks source link

Redirects RFC #587

Closed matthewp closed 1 year ago

matthewp commented 1 year ago

Summary

Provide a redirects config option for specifying redirected pages. Provide good integration with adapters.

Links

tony-sull commented 1 year ago

Notes from May 23 community call:

matthewp commented 1 year ago

Note that this was just updated to explain routing priority. The intent is for it to have the same priority as filesystem routes. If there is an exact match then the redirect would win. But in the case of, for example, siblings with the redirect being a spread, the filesystem non-spread is prioritized, just as it would be if they were both FS based.

matthewp commented 1 year ago

@delucis says:

We likely still need to have a way to avoid building the static routes if the adapter supports redirect config: Netlify will definitely ignore a config redirect in favour of a matching HTML file, and from reading Vercel’s docs, I suspect they do too: “precedence is given to the filesystem prior to rewrites being applied”

I've thought about this and this is a bit problematic. We don't currently have a mechanism for an adapter to prevent a route from being built. A few possible solutions:

  1. Have some sort of special-case option for redirects ala writeRedirects: false. Don't love it.
  2. Provide a way to filter out files before they are written. This is more generic but feels complicated if there's not more use-cases.
  3. The integrations could just delete the files themselves.

I'm leaning towards (3) but if this is something all adapters need to do then that doesn't feel great either.

matthewp commented 1 year ago

config.build.redirects = false wouldn't be terrible either, I guess.

delucis commented 1 year ago

Yeah a config flag adapters can toggle off doesn't seem too bad. A fourth option would be to just do this implicitly when an adapter is set on the basis that if there is an adapter, redirects are their responsibility. But I think a flag might be safest for the case where an adapter doesn't handle this.

matthewp commented 1 year ago

There are likely adapters that do not have config based redirects, so I wouldn't want to blanket disable the feature on the presence of an adapter. I think I'll go with the config approach. Will update the RFC and implementation.

matthewp commented 1 year ago

@delucis Updated to include the config: https://github.com/withastro/roadmap/pull/587/commits/dfd349eefc2251a532b306415ccc320568076e9d

delucis commented 1 year ago

Thanks! LGTM 👍

I think the outstanding points from above are:

matthewp commented 1 year ago
matthewp commented 1 year ago

Updated to specify that FS routes win out when there are exact matches.

delucis commented 1 year ago

I don't think we should widen the value of status. The 200 example seems to be a special case for Netlify

I’m fine with this. I agree sticking to the common features to maintain cross-platform compatibility is a good idea. For the record, something like this is also supported by Cloudflare and Vercel (see https://github.com/withastro/roadmap/pull/587#discussion_r1205878490).

No more comments from me. Thanks for putting up with all my feedback! 😅

matthewp commented 1 year ago

I'd be happy to open this up if we get feedback that people expect this. Will do some more research to see what the support matrix of these codes look like.

jlarmstrongiv commented 1 year ago

We’re having trouble with the new experimental redirects in https://github.com/withastro/astro/issues/7322:

{
  experimental: {
    redirects: true
  },
  redirects: {
    '/a': '/b'
  }
}

I think this syntax matches the RFC proposal

matthewp commented 1 year ago

I'm calling for consensus on this RFC. This will now enter the final comment period. If there are any objections to this feature being unflagged as stable please state so now. After 3 days if there are no objections this RFC will be merged.

matthewp commented 1 year ago

Consensus passes, this RFC is accepted.