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

anchor links have a trailing slash appended when using navigate() with trailingSlash: "always" #36742

Closed
2 tasks done
ezracelli opened this issue Oct 4, 2022 · 4 comments
Closed
2 tasks done
Labels
good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) type: bug An issue or pull request relating to a bug in Gatsby

Comments

@ezracelli
Copy link

Preliminary Checks

Description

this is almost the same issue as #36071, but for the navigate function provided by gatsby. in this case, extra slashes are appended to the URL rather than prepended (e.g. #hash becomes #hash/).


the solution provided in #36071 (using native browser APIs) does extend to programmatic navigation, to some extent; for a hash update that pushes to the history stack and doesn't require updating the history state, it's possible to assign directly to window.location.hash. however, this gets a bit clunky for programmatic navigation using the History API, which is necessary for emulation of the replace or state options of navigate.

this combo means that a simple reactive hash change requires:

window.history.pushState(state, '', hash);
window.dispatchEvent(new PopStateEvent('popstate', { state }));

this pattern (calling navigate() with only a URL hash while using trailingSlash: "always") should be supported out-of-the-box, as Gatsby's own documentation might suggest it is.

Reproduction Link

https://codesandbox.io/s/charming-feynman-frhw59

Steps to Reproduce

  1. set the gatsby-config property trailingSlash to "always"
  2. call navigate("#hash") from anywhere

Expected Result

When calling navigate("#hash"), the page hash should change to #hash

Actual Result

When calling navigate("#hash"), the page hash changes to #hash/

Environment

System:
    OS: macOS 13.0
    CPU: (10) arm64 Apple M1 Pro
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.17.1 - /private/var/folders/q9/63kh50rj6sg1qqhckjzwj3880000gp/T/xfs-1f9f3d4d/node
    Yarn: 4.0.0-rc.22 - /private/var/folders/q9/63kh50rj6sg1qqhckjzwj3880000gp/T/xfs-1f9f3d4d/yarn
    npm: 8.19.2 - ~/.nvm/versions/node/v16.17.1/bin/npm
  Languages:
    Python: 3.10.6 - /usr/local/bin/python
  Browsers:
    Chrome: 105.0.5195.125
    Firefox: 98.0.2
    Safari: 16.1

Config Flags

N/A

@ezracelli ezracelli added the type: bug An issue or pull request relating to a bug in Gatsby label Oct 4, 2022
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 4, 2022
@LekoArts LekoArts added status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 10, 2022
@LekoArts
Copy link
Contributor

This is happening because in

return isAbsolutePath(path) ? applyPrefix(path) : absolutify(path, relativeTo)
the isAbsolutePath is false and then it goes through
function absolutify(path, current) {
// If it's already absolute, return as-is
if (isAbsolutePath(path)) {
return path
}
const option = getGlobalTrailingSlash()
const absolutePath = resolve(path, current)
if (option === `always` || option === `never`) {
return applyTrailingSlashOption(absolutePath, option)
}
return absolutePath
}

It returns with

return applyTrailingSlashOption(absolutePath, option)

It should do the same as

const { pathname, search, hash } = parsePath(prefixed)
const output = applyTrailingSlashOption(pathname, option)
return `${output}${search}${hash}`

We have unit tests for this that should also check this https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-link/src/__tests__/rewrite-link-path.js

@LekoArts LekoArts added help wanted Issue with a clear description that the community can help with. good first issue Issue that doesn't require previous experience with Gatsby labels Oct 10, 2022
@rokaicker
Copy link

Hi @LekoArts, would it be alright if I try to takcle this? I'm relatively new to open source development, but I think this would be a cool thing to work on.

@Prasundas99
Copy link

Hi @LekoArts , If this is open please assign it to me it would like to give it a try

@Prathamesh010
Copy link

Close this. Fixed in #36798

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issue that doesn't require previous experience with Gatsby help wanted Issue with a clear description that the community can help with. status: confirmed Issue with steps to reproduce the bug that’s been verified by at least one reviewer. topic: frontend Relates to frontend issues (e.g. reach/router, gatsby-link, page-loading, asset-loading, navigation) type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants