Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Cloudflare adapter strips the trailing slash, causing the redirect to become a 404 #419

Open
1 task
Khsmty opened this issue Oct 10, 2024 · 3 comments · May be fixed by #521
Open
1 task

The Cloudflare adapter strips the trailing slash, causing the redirect to become a 404 #419

Khsmty opened this issue Oct 10, 2024 · 3 comments · May be fixed by #521

Comments

@Khsmty
Copy link

Khsmty commented Oct 10, 2024

Astro Info

Astro                    v4.15.12
Node                     v22.7.0
System                   Windows (x64)
Package Manager          unknown
Output                   hybrid
Adapter                  @astrojs/cloudflare
Integrations             @astrojs/sitemap
                         @astrojs/tailwind
                         astro-icon

Describe the Bug

Below, I have it set up to redirect from /foo or /foo/ to /bar.
However, when I build this with the Cloudflare adapter, the trailing slash is removed in the _redirects file, and accessing /foo/ results in a 404.

// astro.config.mjs

export default defineConfig({
  redirects: {
    '/foo': '/bar',
    '/foo/': '/bar',
  },

  output: 'server',
  adapter: cloudflare(),
});
# dist/_redirects

/foo    /bar    301
/foo    /bar    301

What's the expected result?

The trailing slash has not been removed from the _redirects file, so it is expected that accessing /foo/ will result in a successful redirect.

# dist/_redirects

/foo    /bar    301
/foo/    /bar    301

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-fzcdgt?file=dist%2F_redirects

Participation

  • I am willing to submit a pull request for this issue.
@mschoeffmann
Copy link

Having the same issue.
My workaround: Create a custom _redirects file in /public directory with correct redirects.
But indeed, a built-in solution would be nicer here.
@ astro team: Do you agree? Would you accept a PR for that?

@alexanderniebuhr
Copy link
Member

I think a PR is welcome. However this needs to be fixed in the underscore redirects package inside the core repository, I guess.

@mschoeffmann
Copy link

mschoeffmann commented Jan 5, 2025

@alexanderniebuhr i found a way to clone the redirects on creation to add a trailing slash.
would this be enough for a PR, or is there more needed?

diff --git a/packages/cloudflare/src/index.ts b/packages/cloudflare/src/index.ts
index 3193683..ad9140f 100644
--- a/packages/cloudflare/src/index.ts
+++ b/packages/cloudflare/src/index.ts
@@ -67,6 +67,12 @@ export type Options = {
 	 * for reference on how these file types are exported
 	 */
 	cloudflareModules?: boolean;
+
+	/**
+	 * Duplicate redirect routes with added trailing slash. Defaults to true.
+	 * When enabled, every entry in _redirects will be duplicated with an added trailing slash.
+	 */
+	redirectAlsoWithTrailingSlash?: boolean;
 };
 
 function wrapWithSlashes(path: string): string {
@@ -342,6 +348,16 @@ export default function createIntegration(args?: Options): AstroIntegration {
 				});
 
 				if (!trueRedirects.empty()) {
+					if (args?.redirectAlsoWithTrailingSlash ?? true === true) {
+						const originalDefinitions = [...trueRedirects.definitions];
+						trueRedirects.definitions = [];
+						for (const definition of originalDefinitions) {
+							trueRedirects.definitions.push(definition);
+							trueRedirects.definitions.push({ ...definition, input: appendForwardSlash(definition.input) });
+						}
+						trueRedirects.minInputLength += 1;
+					}
+
 					try {
 						await appendFile(new URL('./_redirects', _config.outDir), trueRedirects.print());
 					} catch (error) {

this does exactly what is needed: clone the urls with a trailing slash in correct order.
i did this now just for the 'cloudflare' package because i have no experience with other hosters like vercel and do not know if the workaround is needed there, too ...

maybe we could provide the option to the adapter to enable/disable this behaviour? but i'm not sure?
or check which configured redirect has a trailing slash and only add it to these ... ?
in my personal opinion it should be enabled by default - but i'm maybe wrong? :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants