gatsby-remark-autolink-headers doesn't apply offset when clicking on hash link within same page

[alexandernanberg]

Description

If the history updates without the update originating from a Link component onRouteUpdate won't fire, eg. <a href="#some-id">Scroll to some id</a>.

This causes plugins like gatsby-remark-autolink-headers to not work like you would expect when having inline links in the markdown. Try using the links in the TOC and you'll notice that they don't align correctly with the viewport.
https://deploy-preview-1104--reactjs.netlify.com/docs/getting-started.html

Same thing happens on https://next.gatsbyjs.org/docs/migrating-from-v1-to-v2/

Steps to reproduce

• Add a simple anchor tag on the site like <a href="#some-id">Scroll to some id</a> + and element with the id.
• Add a custom onRouteUpdate function.

Notice that it won't fire when clicking the anchor tag.

Expected result

onRouteUpdate should be called.

Actual result

onRouteUpdate isn't called.

Environment

  System:
    OS: macOS High Sierra 10.13.6
    CPU: x64 Intel(R) Core(TM) i7-4870HQ CPU @ 2.50GHz
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 8.11.3 - ~/.nvm/versions/node/v8.11.3/bin/node
    Yarn: 1.9.4 - /usr/local/bin/yarn
    npm: 5.6.0 - ~/.nvm/versions/node/v8.11.3/bin/npm
  Browsers:
    Chrome: 68.0.3440.106
    Firefox: 61.0.2
    Safari: 11.1.2
  npmPackages:
    gatsby: ^2.0.0-beta.64 => 2.0.0-rc.0 
    gatsby-plugin-catch-links: ^2.0.2-beta.5 => 2.0.2-rc.0 
    gatsby-plugin-feed: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-plugin-glamor: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-plugin-google-analytics: ^2.0.0-beta.4 => 2.0.0-rc.0 
    gatsby-plugin-manifest: ^2.0.2-beta.4 => 2.0.2-rc.0 
    gatsby-plugin-netlify: ^2.0.0-beta.6 => 2.0.0-rc.0 
    gatsby-plugin-nprogress: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-plugin-react-helmet: ^3.0.0-rc.0 => 3.0.0-rc.0 
    gatsby-plugin-sharp: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-plugin-twitter: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-remark-autolink-headers: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-remark-code-repls: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-remark-copy-linked-files: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-remark-embed-snippet: ^3.0.0-rc.0 => 3.0.0-rc.0 
    gatsby-remark-images: ^2.0.1-beta.10 => 2.0.1-rc.0 
    gatsby-remark-prismjs: ^3.0.0-rc.0 => 3.0.0-rc.0 
    gatsby-remark-responsive-iframe: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-remark-smartypants: ^2.0.0-rc.0 => 2.0.0-rc.0 
    gatsby-source-filesystem: ^2.0.1-rc.0 => 2.0.1-rc.0 
    gatsby-transformer-remark: ^2.1.1-beta.6 => 2.1.1-rc.0 
    gatsby-transformer-sharp: ^2.1.1-beta.7 => 2.1.1-rc.0 

File contents (if changed)

gatsby-config.js: N/A
package.json: N/A
gatsby-node.js: N/A
gatsby-browser.js: N/A
gatsby-ssr.js: N/A

[Chuloo]

Thanks for sending this in @alexandernanberg 👍

[pieh]

Hey @alexandernanberg what's mostly wrong here is that onRouteUpdate shouldn't actually be called when we navigate to hash link within same page and gatsby-remark-autolink-headers should be adjusted for that.

[DSchau]

@pieh just so we're clear here, you're suggesting that we shouldn't rely on onRouteUpdate to be called (with a hash change), and should instead solve the hash navigation problem differently (i.e. with a custom script or something) in this plugin?

[alexandernanberg]

Wouldn't the best way be to use the Location component from reach/router? Not 100% sure if it's possible to implement it like that in the plugin tho

[DSchau]

Meaning use that instead of the a tag? I don't think that's possible, but we probably can use something from react/router to enable this functionality a little more cleanly.

Here's the relevant snippet where the link is added

[pieh]

I think we can hook into @reach/router history and listen to it in gatsby-browser. Alternatively we can look into shouldScrollUpdate in gatsby-browser, but I don't think that's what we want here

onRouteUpdate should only be called when actual pathname changes (so page changed) - this is for page tracking and stuff like gatsby-plugin-twitter which manipulate DOM after new page has loaded (and DOM was updated)

[alexandernanberg]

No not really, the Location component consumes the router context, like this https://reach.tech/router/api/Location. So I was thinking if we could replace the onRouteUpdate with that, but as @pieh said it's probably better to just listen to the history from the router