gatsbyjs / gatsby

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

[gatsby-plugin-layout] plugin not holding state consistently #16243

Closed herecydev closed 4 years ago

herecydev commented 5 years ago

Description

gatsby-plugin-layout is unmounting and rerendering when using a programmatic navigate between pages. In my layouts/index.js file I am using a useState hook like const [time] = useState(Date.now()) and can observe that occasionally the layout component is rerendered causing the time to be altered.

Demonstration:

In cases when the bug occurs and state is lost, the time would progress: image

In cases when the bug does not occur and state is maintaned, the time would not progress: image

This is absolutely not desirable as the intention is use the plugin to keep state, which is a well documented use case. I can see that when this occurs, the page "flashes white", not acting like a SPA app. When the bug doesn't occur you can see that the layout is not rerendered and the 2nd page component is rendered in a smooth "SPA" fashion.

I am unable to recreate this with <Link />, i.e. seems to be only a navigate problem

Steps to reproduce

Given an absolutely minimal Gatsby app, i.e. starter-hello-world and adding gatsby-plugin-layout and three files:

layouts/index.js

import React, { useState } from "react"

const Layout = ({ children }) => {
  const [time] = useState(Date.now())
  console.log(time)

  return <div>{children}</div>
}

export default Layout

pages/index.js

import React from "react"
import { navigate } from "gatsby"

export default () => <div onClick={() => navigate("/foo")}>Hello World!</div>

pages/foo.js

import React from "react"
import { navigate } from "gatsby"

export default () => <div onClick={() => navigate("/")}>Hello Foo!</div>

Navigating between index and foo will reproduce the layout state issue, the console logs will demonstrate that the time can change

Expected result

State would be preserved between pages when storing in a layout component

Actual result

State is intermittently lost, roughly 25% of the time

Environment

  System:
    OS: Windows 10
    CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
  Binaries:
    Yarn: 1.17.3 - C:\Program Files (x86)\Yarn\bin\yarn.CMD
    npm: 6.9.0 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: 44.17763.1.0
  npmPackages:
    gatsby: ^2.13.45 => 2.13.45 
    gatsby-plugin-layout: ^1.1.2 => 1.1.2 
alexluong commented 5 years ago

This is super interesting/weird. So I tried recreating your problem in codesandbox, and the bug only happens once in the first page change.

However, when I do have another Link component on the page, everything works fine, including changing page with navigate component.

So, gatsby-plugin-layout uses the API wrapPageElement, which basically means a parent component for the pages component.

I tried using wrapRootElement instead and add the date into a context that is used by the page components. On page change, the root component is not re-rendered, so although the white flash still happens, the state is untouched. Maybe that would help your use case? Here is the sample codesandbox for that scenario.

If you haven't already, you can learn more about Browser APIs and SSR APIS.


To sum up, I do think there is a bug somewhere in here.

herecydev commented 5 years ago

Yer I did mention that I couldn't recreate the bug with a Link component, only navigate. Thanks for linking the APIs, I have seen them and the wrapPageElement should be enough but I will try and test with wrapRootElement when I get a chance.

Using React's context like you have is much closer to my "real world example" but I tried to simplify as much as possible.

alexluong commented 5 years ago

Yer I did mention that I couldn't recreate the bug with a Link component, only navigate.

The interesting thing is that if I have a Link component on the page and a button that will navigate on click, everything works fine, including navigate. That's what I'm quite confused about. In my first codesandbox, I have that version commented out. You can try it out for yourself.

herecydev commented 5 years ago

Doesn't Link preload things where as navigate can't as it doesn't know what will be requested?

alexluong commented 5 years ago

Maybe that's the explanation. That sounds very reasonable. To persist data, you would need to wrap it inside wrapRootElement as suggested in the documentation.

herecydev commented 5 years ago

Where does it suggest that in the documentation? wrapPageElement describes:

This is useful for setting wrapper component around pages that won’t get unmounted on page change

Which is what gatsby-plugin-layout uses and says works for "Storing state when navigating pages".

alexluong commented 5 years ago

wrapRootElement

This is useful to setup any Providers component that will wrap your application. For setting persistent UI elements around pages use wrapPageElement.

Does this describe your use case?

herecydev commented 5 years ago

Yes, but so does wrapPageElement

herecydev commented 5 years ago

Any other ideas on this one? Would be good to get a firmer understanding of what's going on and why.

gatsbot[bot] commented 5 years ago

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 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. You can also add the label "not stale" to keep this issue open!

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! 💪💜

herecydev commented 5 years ago

not stale

metervara commented 5 years ago

I'm also experiencing this on Gatsby 2.13.73.

Following alexluong suggestion I have included hidden Link elements for all my routes, which makes navigate work as expected. I'm hiding using visibility: hidden;, I also tried _display:none; but then the problem persists

The reason I'm not using Link directly is because my clicks originate from sub-nodes inside an svg + I'm executing some code on click besides doing the route change

The interesting thing is that if I have a Link component on the page and a button that will navigate on click, everything works fine, including navigate. That's what I'm quite confused about. In my first codesandbox, I have that version commented out. You can try it out for yourself.

LekoArts commented 4 years ago

Is this still the case with latest versions of Gatsby?

github-actions[bot] commented 4 years ago

Hiya!

This issue has gone quiet. Spooky quiet. 👻

We get a lot of issues, so we currently close issues after 30 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. You can also add the label "not stale" to keep this issue open! 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 4 years ago

Hey again!

It’s been 30 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 reopen 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! 💪💜