gatsbyjs / gatsby

The best React-based framework with performance, scalability and security built in.
https://www.gatsbyjs.com
MIT License
55.24k stars 10.32k forks source link

[gatsby-plugin-netlify] Generated _redirects file may lead netlify to apply the wrong rule when there is more than one matchPath that matches a URL #29609

Closed dotboris closed 3 years ago

dotboris commented 3 years ago

Description

Netlify tries to match files from _redirects from top to bottom. This is documented here: https://docs.netlify.com/routing/redirects/#rule-processing-order

If a URL can be matched by more than one matchPath it's important for the rules written in _redirects to be in the right order so that netlify serves the right page.

When gatsby-plugin-netlify generates _redirects it does not care about the order of the rules. Their order ends up being arbitrary. Depending on your luck, the rules may be in the right order or they may be in the wrong order.

If the rules are in the wrong order, netlify will serve the wrong static page. That page will be rendered to the user while gatsby wakes up and hydrates the page. Gatsby will eventually realize that it's rendering the wrong component and render the right one. This leads to the user seeing a flash of the wrong content before getting the right content. This effectively nullifies the performance and UX gains from doing SSR. Also, this might have some big negative impacts on SEO. A crawler will just see the wrong page all together.

I think that this only happens during a hard load of the page. When you navigate within the site, gatsby figures out the routing all by itself and this problem does not occur.

Steps to reproduce

Reproduction:

  1. Clone the repo (when using repo)
  2. yarn install (when using repo)
  3. yarn start (when using repo)
  4. Your browser should open to localhost:8888
  5. Click on the link on the index page (it should land you on /dynamic/thing/42)
  6. Open your dev tools
  7. Turn off the cache
  8. Set throttling to something like Fast 3G
  9. Refresh the page

You will see that the fallback page is rendered at first and then the thing page will be rendered correctly.

Alternatively it's possible to use curl to only see the html of the page. When you do this, it's easy to see that netlify is serving the fallback page.

curl localhost:8888/dynamic/thing/42 | xmllint --format --html - | bat -l html

image

Expected result

I expect to see the dehydrated thing page only which then eventually loads fully

Actual result

I see the fallback page at first, then it's replaced by the thing page in the loading version which eventually loads fully.

Environment

  System:
    OS: macOS 10.15.7
    CPU: (8) x64 Intel(R) Core(TM) i5-1038NG7 CPU @ 2.00GHz
    Shell: 5.7.1 - /bin/zsh
  Binaries:
    Node: 15.6.0 - /var/folders/jq/mvhsh48s765_3m91z7p956fr0000gp/T/yarn--1613753968255-0.6135894534762503/node
    Yarn: 1.22.10 - /var/folders/jq/mvhsh48s765_3m91z7p956fr0000gp/T/yarn--1613753968255-0.6135894534762503/yarn
    npm: 7.4.0 - /usr/local/bin/npm
  Languages:
    Python: 2.7.16 - /usr/bin/python
  Browsers:
    Chrome: 88.0.4324.182
    Firefox: 85.0.2
    Safari: 14.0.3
  npmPackages:
    gatsby: ^2.32.3 => 2.32.4 
    gatsby-plugin-netlify: ^2.11.0 => 2.11.0 
dotboris commented 3 years ago

A possible solution to this would be to try to automatically sort the _redirects file to match the logic that Gatsby uses. I know that behind the scenes Gatsby uses @reach/router for it's routing. I also know that @reach/router has an algorithm where it ranks every rule and disambiguates rules based on those ranks. We could have @reach/router rank those rules for us and then sort them by rank in descending order (highest rank first).

I'm pretty sure that this will just work and generate valid results.

Here's the documentation where the ranking is explained: https://reach.tech/router/ranking The only problem that I'm seeing here is that the code for ranking routes is not exposed by @reach/router. See: https://github.com/reach/router/blob/master/src/lib/utils.js#L243

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

dotboris commented 3 years ago

I want to keep this issue open. I believe that it's an important issue that needs addressing

fivetwelve commented 3 years ago

Hey, I apologize for jumping on this thread but I'm experiencing what feels to me something similar. My issue is happening at https://6054cf449656e0000835d9c7--almex-group.netlify.app/ and I'm using gatsby 2.32.9 with gatsby-plugin-netlify 2.11.1.

I have my issue posted over at Netlify's support forums but am not making headway: https://answers.netlify.com/t/redirect-rules-from-months-no-longer-parsed-the-same-way/33958

In a nutshell, we have a bunch of similar rules that use Netlify's geo-detection to affix the correct region/language to the URL if they're just hitting the domain or a generic promo url where we want Netlify to handle the redirection. What we're also seeing is Gatsby seems to be attempting to load page-data json.data and other chunked code with the geo paths affixed to it as well, instead of loading them from root. Here's an example of our rules:

# Redirect customers for Asia Pacific /* /australia/en/:splat 302 Country=au,kh,ck,fj,pf,jp,kp,kr,la,my,mn,mm,nr,nc,nz,pg,ph,sg,sb,tw,th,vn

and a fallback catch-all for countries we missed or neglected: # Fallback redirect failing all of the above /* /northamerica/en/:splat 301 */

Examples of mixed bag of 200/302/404 requests of data follows. I have a feeling that errors may be occurring with the pre-fetch but I'm not versed well enough to debug why that is beyond seeing examples of the error in the inspector. You'll see in the 404 example it should not be including /northamerica/en in that path.

Screen Shot 2021-03-19 at 2 59 46 PM

Screen Shot 2021-03-19 at 2 59 33 PM

Screen Shot 2021-03-19 at 3 00 04 PM

In any case, I hope the issue I'm experiencing is similar. If someone feels the issue is mutually exclusive, I'm happy to start another one. I don't mean to hijack the thread if my issue is unrelated.

dotboris commented 3 years ago

@fivetwelve I'm not sure that this is related. Do write your redirect rules by hand? From the snippets you posted it seems like you do.

This bug here is about the rules that gatsby-plugin-netlify generates automatically when you're using the following pattern in gatsby-node.js:

createPage({
  ...page,
  matchPath: '/something/*'
})

Specifically, this bug is about how the order of the rules generated by gatsby-plugin-netlify is arbitrary. Your issue seems to either be about how Netlify interprets those rules or how gatsby tries to dynamically load components / page data. If that's the case, then I'm pretty sure that it's unrelated.

fivetwelve commented 3 years ago

@dotboris Thanks for the clarification. I will raise my issue elsewhere.

github-actions[bot] commented 3 years ago

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 60 days of inactivity. It’s been at least 20 days since the last update here. If we missed this issue or if you want to keep it open, please reply here. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks for being a part of the Gatsby community! 💪💜

github-actions[bot] commented 3 years ago

Hey again!

It’s been 60 days since anything happened on this issue, so our friendly neighborhood robot (that’s me!) is going to close it. Please keep in mind that I’m only a robot, so if I’ve closed this issue in error, I’m HUMAN_EMOTION_SORRY. Please feel free to comment on this issue or create a new one if you need anything else. As a friendly reminder: the best way to see this issue, or any other, fixed is to open a Pull Request. Check out gatsby.dev/contribute for more information about opening PRs, triaging issues, and contributing!

Thanks again for being part of the Gatsby community! 💪💜