Site navigation creates increasingly detached DOM nodes

[diegodorado]

Description

When I navigate to another page, new DOM nodes are created, but old one aren't disposed or even cached.

Steps to reproduce

gatsby new gatsby-site
cd gatsby-site

Edit index.js and page-2.js to get as minimal as posible:
// index.js

import React from "react"
import { Link } from "gatsby"
const IndexPage = () => (<Link to="/page-2/">Go to page 2</Link>)
export default IndexPage

// page-2.js

import React from "react"
import { Link } from "gatsby"
const SecondPage = () => (<Link to="/">Go back to the homepage</Link>)
export default SecondPage

gatsby build
gatsby serve

Open localhost:9000 on chrome, and open the Performance Monitor.
Every time you click on the links, DOM Nodes increase (but never gets garbage collected)

Expected result

DOM Nodes should remain more or less steady

Actual result

DOM Nodes keeps increasing on each click

Environment

System:
OS: Linux 4.15 Linux Mint 19 (Tara)
CPU: (8) x64 Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
Shell: 5.4.2 - /usr/bin/zsh
Binaries:
Node: 10.15.3 - ~/.nvm/versions/node/v10.15.3/bin/node
Yarn: 1.16.0 - ~/.nvm/versions/node/v10.15.3/bin/yarn
npm: 6.4.1 - ~/.nvm/versions/node/v10.15.3/bin/npm
Languages:
Python: 2.7.15 - /usr/bin/python
Browsers:
Chrome: 72.0.3626.119
Firefox: 65.0.1
npmPackages:
gatsby: ^2.8.2 => 2.8.2
gatsby-image: ^2.1.2 => 2.1.2
gatsby-plugin-manifest: ^2.1.1 => 2.1.1
gatsby-plugin-offline: ^2.1.1 => 2.1.1
gatsby-plugin-react-helmet: ^3.0.12 => 3.0.12
gatsby-plugin-sharp: ^2.1.3 => 2.1.3
gatsby-source-filesystem: ^2.0.38 => 2.0.38
gatsby-transformer-sharp: ^2.1.21 => 2.1.21
npmGlobalPackages:
gatsby-cli: 2.6.4

[diegodorado]

Here's a gif showing this behaviour

[KyleAMathews]

Hmmm that's no good. We've got a memory leak somewhere...

…

On Mon, Jun 3, 2019, 5:07 PM Diego Dorado ***@***.***> wrote: Here's a gif showing this behaviour [image: test] <https://user-images.githubusercontent.com/208685/58842214-8db6a980-8643-11e9-9fde-0129fa0d5952.gif> — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#14525?email_source=notifications&email_token=AAARLB7IXRQXKAV5MXVSKATPYWW5VA5CNFSM4HSVRNQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW3AZ6A#issuecomment-498470136>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAARLB3QW5BPKF2GJUJMIJTPYWW5VANCNFSM4HSVRNQA> .

[phacks]

This has been discussed in a previous issue for reference: #12198

I’m willing to work on this, any hints to where I should begin with?

[phacks]

Will try and work on it this weekend!

[diegodorado]

Will try and work on it this weekend!

Let me know if I can be of any help.

[phacks]

Sooo, I dug around a bit, created a repro repo, managed to get some insights with the Chrome Dev Tools, however it appears my knowledge of the inner workings of Gatsby isn’t deep enough to know where to start 😅

What I did:

• Created a repro repository following @diegodorado’s repro: https://github.com/phacks/gatsby-memory-leak-repro. Bumped the number of <a>s to 10 so that it’s easier to see the impact.
• Played around with snapshots, and I noticed that the increased DOM size seems to come from “Detached DOM Nodes” :

Before clicking on a link:

After clicking on a link:

Per this SO answer, Detached DOM Nodes are:

HTML elements are instance of objects that consume memory. Each such element can store on them event listeners and data associated with it. Now "Detached DOM Trees" are nothing but DOMs that are in the Browser's memory but are NOT attached to the main DOM tree aka "Document DOM Trees".

Hopefully this may help @diegodorado or someone else have some ideas about where to start!

[oorestisime]

I wonder whether this issue is related to facebook/react#15157

[phacks]

@oorestisime Interesting issue, nice catch!

When I patch, in the repro, react-dom by the discordapp#react suggested dirty fix (mentioned in facebook/react#15157 (comment)), no “Detached DOM Nodes” appear in subsequent snapshots, so it seems to be doing something.

However, the DOM Node counter is still increasing, which gets me even more perplex.

I updated the repro repo (https://github.com/phacks/gatsby-memory-leak-repro) to explain how to reproduce (and how to visualise) the leak, so hopefully it’ll be easier to help along.

To test out Discord’s hotfix, you may change the following in the package.json:

- "react-dom": "^16.8.6",
+ "react-dom": "discordapp/react#cfda84f6b3c49a1398709cf43b3b959366f7e01a",

[gatsbot]

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/contributefor more information about opening PRs, triaging issues, and contributing!

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

[phacks]

Looks like a memory leak was fixed in React very recently (see this long discussion about the topic).

I’ll try and have a look this week to see whether that fixes the problem in Gatsby!

[phacks]

Update: tried running the example repo against the master branch of react and react-dom, and it did not fix the memory leak.

[diegodorado]

Wow... seems to be hard to fix... does this means that this is not a react issue but a gatsby issue?

…

On Sun, Jul 28, 2019, 10:10 Nicolas Goutay ***@***.***> wrote: Update: tried running the example repo <https://github.com/phacks/gatsby-memory-leak-repro> against the master branch of react and react-dom, and it did not fix the memory leak. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14525?email_source=notifications&email_token=AABS6LI2OCFRIP4IYWRD7O3QBWLCTA5CNFSM4HSVRNQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD266LPI#issuecomment-515761597>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABS6LONA2UT5YZHFUHGWLTQBWLCTANCNFSM4HSVRNQA> .

[RyKilleen]

Jumping in to take a look at the repro repo to see if I can glean some insight from it! I'll keep a running update as an edit of this comment for now.

Update 1
I'm noticing that there's a direct correlation to how many intersection observers are also detached, as well as event listeners related to the intersection observers, screenshot below.

If I had to guess, it has something to do with GatsbyLink's refs or createIntersectionObserver method holding on to a reference of an element somewhere.

[phacks]

Oops, I fixed the IntersectionObserver leak a while back but completely forgot to update this issue!

Your insight is spot on, you can see how it was fixed in this PR: #17056

I’ll close the issue now to avoid further confusion, however if there’s another leak in there we can totally reopen it. Sorry about that @RyKilleen !

[RyKilleen]

@phacks no worries, it was fun to dig in! Glad to know my hunch was in the right direction!