Gatsby: matchPath priority takes over statically generated pages after version 2.15.22 and up to 2.17.4

[antoinerousseau]

Description

I generate events with the createPage action using an event.js template. But not all events are pre-generated using this method, so I also have an event.js page that has a matchPath = '/:year/:month/:day/:slug' to fallback and dynamically display unknown events.
This was working great, up to Gatsby version 2.15.22. This is what runs at https://lebikini.com

For example this is pre-generated:
https://lebikini.com/2019/12/10/tiken-jah-fakoly/

And this is not:
https://lebikini.com/2019/10/03/deluxe-sora/

The idea is to not generate all past events as it is resource-intensive.

But with the recent modifications in Gatsby about page metadata, this stopped working in my case, so I locked the Gatsby version at 2.15.22.

Then I tried to upgrade again yesterday but even #18478 published in 2.17.3 does not fix it.

Steps to reproduce

Minimum reproducible example using the default starter:
https://github.com/antoinerousseau/gatsby-bug/commit/671ae5e425d9e9fc8db61b30bec8ca7f22422b34

You can see it in action here:
https://gatsby-bug.netlify.com/2019/10/26/test-event/

Expected result

We should see the static page created using the event template.

Actual result

We see the fallback page because of its matchPath.

Environment

  System:
    OS: macOS 10.14.6
    CPU: (8) x64 Intel(R) Core(TM) i5-8259U CPU @ 2.30GHz
    Shell: 5.3 - /bin/zsh
  Binaries:
    Node: 10.15.3 - ~/.nvm/versions/node/v10.15.3/bin/node
    Yarn: 1.19.1 - ~/.nvm/versions/node/v10.15.3/bin/yarn
    npm: 6.12.0 - ~/.nvm/versions/node/v10.15.3/bin/npm
  Languages:
    Python: 2.7.16 - /usr/local/bin/python
  Browsers:
    Chrome: 77.0.3865.120
    Firefox: 69.0
    Safari: 13.0.2
  npmPackages:
    gatsby: ^2.17.4 => 2.17.4
    gatsby-image: ^2.2.29 => 2.2.29
    gatsby-plugin-manifest: ^2.2.23 => 2.2.23
    gatsby-plugin-offline: ^3.0.16 => 3.0.16
    gatsby-plugin-react-helmet: ^3.1.13 => 3.1.13
    gatsby-plugin-sharp: ^2.2.32 => 2.2.32
    gatsby-source-filesystem: ^2.1.33 => 2.1.33
    gatsby-transformer-sharp: ^2.3.0 => 2.3.0

Related issues

18604

17980

[pieh]

Yeah, we discovered that PR that we merged is not full solution (it handles just * wildcard and not dynamic params syntax like one you use).

One thing that @sever1an (author of PR you referenced) raised in discord chat we had about this is that @reach/router already have scoring mechanism outlined in https://reach.tech/router/ranking that we could reuse instead of implementing our own custom scoring. Unfortunately it doesn't export it (it's an internal API - https://github.com/reach/router/blob/28a79e7fc3a3487cb3304210dc3501efb8a50eba/src/lib/utils.js#L216-L254 ), but we could copy & paste it (it wouldn't end up in browser bundle so duplicated code, while not great, is not deal breaker)

[antoinerousseau]

@pieh or maybe ask the reach team to export it?

[antoinerousseau]

Anyway, shouldn't the pages having a matchPath arrive after the ones without one?
As stated in the doc actually:

Note that the pages are sorted so that those with matchPaths are at the end, so that explicit paths are matched first.

[antoinerousseau]

Replacing matchPath = '/:year/:month/:day/:slug' with matchPath = '/*/*/*/*/' does solve the issue indeed, fyi.

Edit: it actually acts as a catch-all like the 404, catching pages like /i-dont-exist/ even though it has less slashes, so... not good

[pieh]

Anyway, shouldn't the pages having a matchPath arrive after the ones without one?
As stated in the doc actually:

Note that the pages are sorted so that those with matchPaths are at the end, so that explicit paths are matched first.

Are you referring to https://www.gatsbyjs.org/docs/gatsby-internals-terminology/ page? If so, see note on the top of page - this doc wasn't updated after lot of internal refactoring and we no longer have single page manifest where this was working like that :( We still have partial pages manifest that handle matchPath routes, but because it works differently than previous implementation a lot of those notes do not apply anymore

[antoinerousseau]

@pieh gotcha, didn't see it since the anchor got me straight to the paragraph from google, but thanks

[sever1an]

Perhaps we could copy and paste the scoring function from reach-router pending their decision whether or not to export it? Using this would guarantee that behavior is always consistent with <Router>.

[antoinerousseau]

Sounds good!
I'll see if I find time to make a PR, unless you or someone else has a better grasp of it to make it.

[pieh]

I'll open PR in a sec with early version of it - I didn't try running it on reproduction

[pieh]

Yay - it works with the changes in PR:

I need to add actual test cases to our e2e tests - will use one from your reproduction and will also try to incorporate ones from https://github.com/sever1an/match-path-error/tree/root-error and https://github.com/sever1an/match-path-error/tree/folder-error (that were showing on wildcard routes)