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

[Docs] Anchor parameter is stripped out of redirected links part 1 #2139

Closed
3 of 7 tasks
aswanson-nr opened this issue May 5, 2021 · 44 comments · Fixed by gatsbyjs/gatsby#32334 or gatsbyjs/gatsby#32850
Closed
3 of 7 tasks
Assignees

Comments

@aswanson-nr
Copy link
Contributor

aswanson-nr commented May 5, 2021

Related Gatsby Issue

gatsbyjs/gatsby#27582

Release Date

We merged gatsbyjs/gatsby#32334 and released it in gatsby@next so you could try it out there. It'll then be released on July 20th in 3.10

Description

The redirects generated by the gatsby-plugin-meta-redirect plugin do not remove the trailing slash. This causes any #anchor parameters to be stripped out of the link and most likely applies to query parameters as well.

Steps to reproduce

  1. Go to https://docs.newrelic.com/docs/insights/new-relic-insights/using-new-relic-query-language/nrql-reference#functions)[https://docs.newrelic.com/docs/insights/new-relic-insights/using-new-relic-query-language/nrql-reference#functions
  2. Notice that the page redirects
  3. Notice that the #functions was removed from the URL and the page did not scroll down to that section

Expected

When a user clicks on the link in the example above, they should land on the Functions section of the page.

Screen Shot 2021-05-06 at 9 25 51 AM

Other information

Acceptance Criteria

  • Test using Gastby next release
  • If tests pass, upgrade to Gatsby 3.10 when available.
  • Upgrade Developer SIte
  • Merge Developer develop branch in to feature/o11y-packs
  • Upgrade OSS site
  • Upgrade private docs
  • Upgrade Theme
@aswanson-nr aswanson-nr added bug a bug or issue eng issues related to site functionality that requires engineering labels May 5, 2021
@austin-schaefer
Copy link
Contributor

Thanks @aswanson-nr . There are a lot of redirects on the site and a lot of anchor links, so a fix here will be very helpful!

@jpvajda jpvajda added the P2 label May 6, 2021
@jpvajda jpvajda added this to the [DevEn] Sprint 4 milestone May 6, 2021
@jpvajda
Copy link

jpvajda commented May 6, 2021

from @zstix I believe those redirects come from https://www.gatsbyjs.com/docs/reference/config-files/actions/#createRedirect in Gatsby. Not sure if we have the option to pass along the extra URL params

@rudouglas rudouglas self-assigned this May 11, 2021
@rudouglas rudouglas added the sp:2 label May 11, 2021
@jpvajda jpvajda added sp:5 and removed sp:2 labels May 11, 2021
@jpvajda
Copy link

jpvajda commented May 11, 2021

We netted out that this is still a problem in Sprint Planning today

@rudouglas rudouglas removed their assignment May 12, 2021
@moonlight-komorebi moonlight-komorebi self-assigned this May 17, 2021
@moonlight-komorebi
Copy link
Contributor

moonlight-komorebi commented May 17, 2021

@jpvajda it looks like we can't implement this easily since gatsby doesn't have support for this currently (see here).

is it cool if i close this ticket out?

@jpvajda
Copy link

jpvajda commented May 17, 2021

I'm going to bump Gatsby about it, and try to get an ETA of when this may be available in Gatsby.

@pieh
Copy link
Contributor

pieh commented May 19, 2021

Hey @jpvajda! We published today new release (gatsby@3.5.1), I'm currently veryfying it and will open PR with bump if all things look good.

Ops, sorry I was meant to respond to different issue - sorry for confussion

@rudouglas
Copy link
Contributor

This may be a moot point now if the Gatsby update fixes this, but I figured out a workaround for this, i've got this working in develop build successfully, but for some reason it's not working in production, i'm confused about why these would work differently so it would be good to learn more about the redirection process.

Here's the branch with the changes I made: https://github.com/newrelic/docs-website/tree/ruairi/anchor-redirect

I used sessionStorage and a few of the Browser API's to achieve this.

screen-recording-2021-05-19-1502.webm.mp4

Maybe it has something to do with the fact that it throws a 404 first in develop

Notes:
The Issue linked by Alec seems to be separate to this but does seem to be an issue: nsresulta/gatsby-plugin-meta-redirect#11

@pieh
Copy link
Contributor

pieh commented May 19, 2021

I want to correct myself - I meant to write this to different (but still related to redirects) issue ( #1874 ). For forwarding query params and hash, we didn't implement that in Gatsby yet

@jpvajda jpvajda removed the sp:3 label Oct 20, 2021
@jpvajda jpvajda changed the title [Gatsby Core] Anchor parameter is stripped out of redirected links [Docs] Anchor parameter is stripped out of redirected links Oct 20, 2021
@austin-schaefer
Copy link
Contributor

Hey @roadlittledawn , let's look at this for a sustaining ticket in the next sprint or two.

@jpvajda
Copy link

jpvajda commented Nov 4, 2021

We have a sustaining epic planned in January to kick off Q4, this is a good candidate for that epic, so added it.

@roadlittledawn
Copy link
Collaborator

roadlittledawn commented Nov 19, 2021

looks like this was fixed in gatsby v3.13, so upgrading to this may fix this? https://www.gatsbyjs.com/docs/reference/release-notes/v3.13/#notable-bugfixes--improvements

gatsbyjs/gatsby#32850

@jpvajda
Copy link

jpvajda commented Nov 22, 2021

We are planning an upgrade to gatsby 4 for the docs site in our sustaining sprint, that should take care of this I believe.

@rudouglas
Copy link
Contributor

@roadlittledawn I did try that when it was initially released but it didn't seem to fix the issue

@jpvajda jpvajda self-assigned this Dec 3, 2021
@jpvajda
Copy link

jpvajda commented Dec 3, 2021

closing for a nice fresh issue out of the oven 🍪 as this has so much history, we'll work from a new issue to resolve this soon

@jpvajda jpvajda closed this as completed Dec 3, 2021
@jpvajda
Copy link

jpvajda commented Dec 3, 2021

#5163

@jpvajda jpvajda added duplicate and removed sustaining bug a bug or issue eng issues related to site functionality that requires engineering labels Dec 3, 2021
@jpvajda jpvajda changed the title [Docs] Anchor parameter is stripped out of redirected links [Docs] Anchor parameter is stripped out of redirected links [part 1] Dec 3, 2021
@jpvajda jpvajda changed the title [Docs] Anchor parameter is stripped out of redirected links [part 1] [Docs] Anchor parameter is stripped out of redirected links part 1 Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment