routes with and without trailing forward slash causes route "misses"

[thescientist13]

Type of Change

• New Feature Request
• Documentation / Website
• Improvement / Suggestion
• Bug
• Other (please clarify below)

Summary

As evidenced in PR #308, it looks like if there is or isn't a forward slash at the end of a route. e.g.

http://domain.com/about

vs.

http://domain.com/about/

Can lead to issues with matching against active routes in GraphQL / coming out of the Greenwood graph and even cause errors in the console, although I think Netlify will add / forward to the / so then it "catches".

Details

I think anywhere where we are matching against an active route or using window.location.hostname or similar to find out the current page, we will need to evaluate how it is being implemented and try and build a fairly robust / non fragile solution.

Also see #310 for reference.

[thescientist13]

Well, now adding the forward slash removes the undefined error, but adds this error now instead 😞 🤷
ex: https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/about/how-it-works/

index.b37c464fdfd3e49a05d7.bundle.js:1 Uncaught (in promise) Invariant Violation: Invariant Violation: 8 (see https://github.com/apollographql/invariant-packages)
    at new e (https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:1:16081)
    at https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:117:58147
    at Array.forEach (<anonymous>)
    at n.diffQueryAgainstStore (https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:117:58105)
    at n.readQueryFromStore (https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:117:57465)
    at e.read (https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:117:67395)
    at e.n.readQuery (https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:117:47470)
    at https://deploy-preview-284--elastic-blackwell-3aef44.netlify.app/index.b37c464fdfd3e49a05d7.bundle.js:117:73775

Note: our site doesn't add the / so normal browsing should be fine, but in my case when forcing it, I noticed the above. One step forward, one step back. 👣

cc: @hutchgrant

[hutchgrant]

For our site, the solution on netlify is:

Settings > Build & deploy > Post processing > Asset optimization then disable asset optimization and then disable pretty urls

Which eliminates forcing the trailing slash.

The problem is that we're caching routes without the trailing / and therefore when we visit URLs with the trailing / our cache does not contain a matching route which gives us these errors when it hydrates the graphql query.

Quick solution is to stop forcing trailing slash from netlify to begin with; its unnecessary.

We can probably close this issue because this is a special case and we cannot accommodate everyone's url rewrite rules. For netlify, maybe we can document this fix somewhere, maybe in build/deploy ?

Index pages will always have a trailing / and every other page will not have a trailing slash. This is assuming that the host doesn't rewrite the links (pretty urls via netlify). This is clear within the graph.js lifecycle:

// get md file's name without the file extension
let fileRoute = subDir.substring(seperatorIndex, subDir.length - 3);

// determine if this is an index file, if so set route to '/'
let route = fileRoute === '/index' ? '/' : fileRoute;

We do this so that we don't have links such as /docs/index and instead have /docs/. All pages within /docs/ won't have a trailing / e.g. /docs/markdown

[hutchgrant]

We could also eliminate trailing slash from index page routes as well.

// determine if this is an index file, if so set route to '/'
let route = fileRoute === '/index' ? '' : fileRoute;

The issue with that is:

if you have a file /docs/index.md it would resolve to /docs while currently it resolves to /docs/.
if you also have a file /docs.md it would also resolve to /docs in both cases

We would have a duplicate route pointing to different pages.

[thescientist13]

Good call @hutchgrant on Netlify settings .

Applied this setting for now, seems to have fixed the issue. 💯

Let me add this issue to our Tuesday agenda to make sure there's nothing else we should be doing and / or documenting.

[thescientist13]

So the question is: What should our default link format be, with or without a forward slash; /, and that either way it should be configurable, most likely from greenwood.config.js.

[thescientist13]

From this PR, maybe anywhere we have to do some sort of matching, we should use a RegExp?

const route = window.location.pathname;

/* get some pages from GraphQL */

const currentPage = response[1].data.graph.filter((page) => {
  return new RegExp(`^${path}$`).test(list.item.link);
})[0];

Then we could only have /docs and it should still match if a route is /docs/? Then maybe no configuration? But maybe it would still be good to configure the / somehow? Not sure... 🤔

[thescientist13]

Hmm, now i'm getting the cache invariants locally using the serve command. 😞

Maybe an issue with the local web server prettifying URLs?

[thescientist13]

I've normalized all routes as ending with / as part of #449