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

Trailing slash redirect broken when using path prefix #39043

Open
2 tasks done
patrickdemers6 opened this issue Jul 13, 2024 · 4 comments · May be fixed by #39164
Open
2 tasks done

Trailing slash redirect broken when using path prefix #39043

patrickdemers6 opened this issue Jul 13, 2024 · 4 comments · May be fixed by #39164
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby

Comments

@patrickdemers6
Copy link

patrickdemers6 commented Jul 13, 2024

Preliminary Checks

Description

A project using pathPrefix and trailingSlash does not redirect properly.

When a page is loaded and requires a redirect due to the slash, the redirect does not take pathPrefix into account.

Reproduction Link

https://github.com/patrickdemers6/gatsby-slash-redirect-path-prefix-bug

Steps to Reproduce

  1. Set pathPrefix to anything.
  2. Set trailingSlash to never.
  3. Run with gatsby build --prefix-paths and gatsby serve --prefix-paths
  4. Load a page with trailing slash.

Example: pathPrefix = /prefix

Expected Result

The redirect removes just the trailing slash.

Expected /prefix/hello/ => /prefix/hello

Actual Result

The redirect removes the slash and the pathPrefix.

Actual /prefix/hello/ => /hello

Environment

System:
    OS: macOS 14.5
    CPU: (12) arm64 Apple M2 Pro
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.17.0 - ~/.nvm/versions/node/v18.17.0/bin/node
    Yarn: 1.22.22 - ~/.nvm/versions/node/v18.17.0/bin/yarn
    npm: 9.6.7 - ~/.nvm/versions/node/v18.17.0/bin/npm
  Browsers:
    Chrome: 126.0.6478.127
    Safari: 17.5
  npmPackages:
    gatsby: ^5.14.0-next.4 => 5.14.0-next.4
  npmGlobalPackages:
    gatsby-cli: 5.13.3

Config Flags

No response

@patrickdemers6 patrickdemers6 added the type: bug An issue or pull request relating to a bug in Gatsby label Jul 13, 2024
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 13, 2024
@rosszurowski
Copy link

I was running into this too. I noticed that Gatsby seems to be dead and while in the long run, I expect to move away from Gatsby, but in the short-term, I needed a fix.

Issue

It seems like the source of the bug is in gatsby-link, where several code paths don't call withPrefix. I cloned the gatsby repo, and modified the gatsby-link package to add the appropriate lines. (Confusingly, this file seems to have an isAbsolutePath function which returns true for relative paths).

diff --git a/packages/gatsby-link/src/rewrite-link-path.js b/packages/gatsby-link/src/rewrite-link-path.js
index e1b2b49671..6668b76c9b 100644
--- a/packages/gatsby-link/src/rewrite-link-path.js
+++ b/packages/gatsby-link/src/rewrite-link-path.js
@@ -13,25 +13,26 @@ const getGlobalTrailingSlash = () =>
 function applyTrailingSlashOptionOnPathnameOnly(path, option) {
   const { pathname, search, hash } = parsePath(path)
   const output = applyTrailingSlashOption(pathname, option)
 
   return `${output}${search}${hash}`
 }
 
 function absolutify(path, current) {
   // If it's already absolute, return as-is
   if (isAbsolutePath(path)) {
-    return path
+    return withPrefix(path)
   }
 
+  const prefixed = withPrefix(path)
   const option = getGlobalTrailingSlash()
-  const absolutePath = resolve(path, current)
+  const absolutePath = resolve(prefixed, current)
 
   if (option === `always` || option === `never`) {
     return applyTrailingSlashOptionOnPathnameOnly(absolutePath, option)
   }
 
   return absolutePath
 }
 
 function applyPrefix(path) {
   const prefixed = withPrefix(path)

Fixing It

The site I'm working on uses patch-package to apply the above change to Gatsby link. To generate this yourself, you can clone the repo, apply the above change, build, and then generate a patch from that.

If you'd rather re-use my work, you can copy the patch file in this gist to patches/gatsby-link+5.10.0.patch in your repository.

I haven't tested this too thoroughly, but it seems to handle all the cases I've thrown at it so far.

@serhalp
Copy link
Contributor

serhalp commented Nov 18, 2024

Thank you for sharing your solution with the community @rosszurowski!

Though we aren't actively investing in new features or low-priority bug fixes, Gatsby is still maintained.

If you open a PR with your bug fix, we'll take a look and get it merged so everyone can benefit! Thanks 🙂

@serhalp serhalp added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Nov 18, 2024
@rosszurowski
Copy link

Hi @serhalp — good to know! I didn't see an official response in that thread, so assumed it was accurate. I'll gladly open a PR with the fix.

@serhalp
Copy link
Contributor

serhalp commented Nov 18, 2024

Awesome, thank you!

For the record, there is a response here and a blog post here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
3 participants