turbolinks:before-render for unmounting causes warnings #607

thebadmonkeydev · 2016-09-30T09:07:04Z

Help us help you! Please choose one:

My app crashes with react-rails, so I've included the stack trace and the exact steps which make it crash.
My app doesn't crash, but I'm getting unexpected behavior. So, I've described the unexpected behavior and suggested a new behavior.
I'm trying to use react-rails with another library, but I'm having trouble. I've described my JavaScript management setup (eg, Sprockets, Webpack...), how I'm trying to use this other library, and why it's not working.
I have another issue to discuss.

This change ecfdc52 has been causing warnings in the console for me. I've followed the standard setup process described in the README. I'm using turbolinks 5 on a new rails 5 app.

When navigating from a page with a react component to one that does not have one and then back I receive the following warning:

react.self-233f9d8….js?body=1:20479 Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React.

I can resolve the error by adding this to my application.js file:

ReactRailsUJS.handleEvent('turbolinks:before-cache', function() {
  window.ReactRailsUJS.unmountComponents();
);

essentially undoing this

The text was updated successfully, but these errors were encountered:

benoitongit · 2016-12-12T00:26:11Z

Exact same issue here:
Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React.
Any idea how to fix this without having to override ract-rails code?

It looks like it's related to Turbolinks cache. If cache is disabled, it fixes this issue:

<head>
  ...
  <meta name="turbolinks-cache-control" content="no-cache">
</head>

rmosolgo · 2016-12-12T02:45:11Z

I bet Turbolinks changes the whitespace when caching the content.

I think the right thing to do is unmount before caching, as suggested by the OP. It will make it work, but we lose the nice UX of #521.

Hmm, I'm not sure how to get both!

benoitongit · 2016-12-12T02:54:30Z

I tried with before-cache, but as you mentioned UX is not as nice. Elements jump around a bit...
Ideally, we can keep it like that and somehow remove the warning :)

benoitongit · 2016-12-13T01:19:53Z

I've been testing today without unmounting with Rails 5 and Turbolinks 5. It works like a charm, no warning and I can't find any issue. It looks like Trubolinks is doing the cleanup since it uses cloneNode(true) for caching.

I am probably missing something, any reason why we have to unmount components here?

setup: function() {
  ReactRailsUJS.handleEvent('turbolinks:load', function({window.ReactRailsUJS.mountComponents()});
  //ReactRailsUJS.handleEvent('turbolinks:before-cache', function({window.ReactRailsUJS.unmountComponents()});
}

rmosolgo · 2016-12-13T03:44:33Z

Imagine you have a React component for which, as long as it is mounted, it is pinging the server for updates.

We need a way to stop the ping. Usually that's with componentWillUnmount. Even if pinging for updates is a bad idea, event listeners or unfinished AJAX calls need the same treatment.

Is there another way to address those issues?

benoitongit · 2016-12-13T06:29:20Z

I see your point and it makes sense at some level. I am just wondering if this is the role of react-rails to take care of this case. It depends on which angle we are looking at this problem, but if you are building a SPA app, it is important to avoid memory leaks. But if you are not, it's not as important since the all page get flushed when browsing to the next page.

Also, when using React framework directly it's up to the developer to call unmountComponentAtNode(), maybe it could be the same here.

Either way is fine with me, I just wish we can find a clean way to unmount components without having to do it in before-cache.

For event listeners cloneNode() doesn't copy event listeners has mention in the doc, so that should be OK.

Thanks for your amazing work @rmosolgo on this.

thebadmonkeydev · 2016-12-13T17:23:54Z

Wow, didn't expect this one to resurface, thanks!! 😃

Is there a way to suppress the warnings? In my case at least like @benoitongit mentioned, I'm not building an SPA so I'm not as worried about leaks in that way.

rmosolgo · 2016-12-13T19:06:35Z

SPA

Isn't a Turbolinks front end basically a single-page app? Since Turbolinks replaces the <body /> in place, your JavaScript state remains intact from page to page. If we don't call componentWillUnmount and friends, you can have memory leaks with Turbolinks!

benoitongit · 2016-12-13T20:30:53Z

To me it's not a SPA, it's an hybrid version to get the responsiveness of a SPA, as they say on turbolinks github:

Get the performance benefits of a single-page application without the added complexity of a client-side JavaScript framework

I don't think you will see the following happen in a SPA:

Turbolinks saves a copy of the current page to its cache just before rendering a new page. Note that Turbolinks copies the page using cloneNode(true), which means any attached event listeners and associated data are discarded.
source

The root of this issue is Turbolinks caching system. As we've seen, without it, react-rails works perfectly as it is currently. Adding turbolinks cache and then we see the warning.

Unsure about the memory leaks with Turbolinks, it would have to be tested to see if it happens or not..

benoitongit · 2016-12-14T02:35:10Z

After more digging, looks like @rmosolgo you're right. Unmounting is important with Turbolinks to avoid memory leak.
Navigating from page A -> B -> C and looking with the React dev tool, components in page A are still hanging there in memory while in page C.

perezperret · 2017-02-01T15:20:51Z

Looking into this, I think the ideal solution would be to "unbind" the component without touching the markup, thus cleaning up any memory use but preventing the flicker, and complying with the turbolinks cache, but I haven't found a simple way to do this with React or ReactDOM. Maybe we could use ReactDOMServer to render and append the markup again without a mounted component. On the other hand we could add a fallback to attach the listener to 'turbolinks:before-render' if cache is disabled. All this feels like overkill, but it should leave everything in working order

wdiechmann · 2017-02-03T13:07:14Z

hmmm - my 2c thinks to look 'the competition' over the shoulder ;)

shakacode/react_on_rails@1918345

perhaps that could bring us in the clear?

szyablitsky · 2017-02-03T13:39:15Z

I think the best solution is to modify ReactDOM's unmountComponentAtNode and more specifically unmountComponentFromNode so it can bypass emptying of the container https://github.com/facebook/react/blob/8791325db03725ef4801fc58b35a3bb4486a8904/src/renderers/dom/stack/client/ReactMount.js#L190
May be additional boolean parameter like doNotEmptyContainer

szyablitsky · 2017-02-04T04:36:00Z

renamed parameter to shouldLeaveMarkup and filled a pull request to React
facebook/react#8928

szyablitsky · 2017-02-06T06:34:53Z

I get an answer about my PR from Dan Abramov. He rightly asked why we get an error about 'another copy of React' in first place. I can not reproduce 'another copy of React' warning by any means. The only way I was able to get this warning is to do a modification in component which leads to an error while rendering component when state was changed wrong way.
But I get this warning after rendering error either way with turbolinks:before-render and turbolinks:before-visit (turbolinks:before-cache)

Can anyone provide me with exact reproducible scenario how to get this warning only in case of turbolinks:before-render so I can dig deeper?

renchap · 2017-02-26T09:58:23Z

@szyablitsky it looks like @sevos found out how to fix this problem while adding Turbolinks support for webpacker-react: renchap/webpacker-react#14 (comment)
You may want to try the same fix :)

borisrorsvort · 2017-03-14T08:30:40Z

They also fixed it like suggested in the ticket on the react-on-rails project shakacode/react_on_rails@1918345

cc @rmosolgo

szyablitsky · 2017-03-15T03:01:06Z

@borisrorsvort this change is already reverted back to before-render
shakacode/react_on_rails#706
https://github.com/shakacode/react_on_rails/blob/master/node_package/src/clientStartup.js#L187

borisrorsvort · 2017-03-15T06:30:54Z

@szyablitsky Thanks for the clarification. Yet the problem persists with before-render in react-rails 1.10. Do you have any insights about where we should look?

szyablitsky · 2017-03-15T06:36:17Z

@borisrorsvort I think you should look at renchap/webpacker-react#14 (comment) and https://sevos.io/2017/02/27/turbolinks-lifecycle-explained.html as @renchap advised.

szyablitsky · 2017-03-15T06:38:00Z

It seems that it's more likely mount event issue (not unmount).

sevos · 2017-03-15T06:43:38Z

Currently in your gem you listen on turbolinks:load

        document.addEventListener('turbolinks:load', reactOnRailsPageLoaded);

As I explained in my article, this event pairs up nicely only with before-cache event.

In case you want to make use of turbolinks' caching, you should listen to this event once, and then listen to render event:

document.addEventListener('turbolinks:load', reactOnRailsPageLoaded, {once: true})
document.addEventListener('turbolinks:render', reactOnRailsPageLoaded)
document.addEventListener('turbolinks:before-render', reactOnRailsPageUnloaded)

szyablitsky · 2017-03-15T07:46:12Z

once option is not widely adopted by current browsers, so I'd recommend using explicit removal of 'turbolinks:load' listener on first event handler call
http://caniuse.com/#search=once

sevos · 2017-03-15T10:20:35Z

Alternatively (from comments under my article):

document.addEventListener("DOMContentLoaded", reactOnRailsPageLoaded)
document.addEventListener('turbolinks:render', reactOnRailsPageLoaded)
document.addEventListener('turbolinks:before-render', reactOnRailsPageUnloaded)

borisrorsvort · 2017-03-15T10:24:36Z

@rmosolgo @hrishimittal Do you expect someone to make a pr or are you already working on a fix?

leonelgalan · 2017-04-10T23:35:09Z

I tried @sevos solution and, for me, it only changes the warning from:

Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React.

to

Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the MyComponent r component.

I also noticed that the warnings (both) don't happen when using the Back and Forward arrows (I assume the page is loaded from Turbolinks cache), but they do when clicking on a link (even when the page was visited before, same page I could simply Back into). When the warnings happen, their is a visible "reload" of the page, a previous version of the page loads first, instantaneously followed by a longer load that actually hits the server.

I'll try to reproduce the issue in a simpler/smaller app.

Update 1

First warning reproduced in https://github.com/leonelgalan/turbolinks-react-rails-example/tree/v1, by clicking into the "Two" link, followed by clicking on "One". @sevos fix, removes that first warning, you can see this in https://github.com/leonelgalan/turbolinks-react-rails-example/tree/v2.

I haven't been able to reproduce the second warning, my example components are too simple. I'm working on that right now.

Update 2

I've reproduce the other warning I was getting in https://github.com/leonelgalan/turbolinks-react-rails-example/tree/v3 and I seem to found the cause, a double render of view. See details on the README o https://github.com/leonelgalan/turbolinks-react-rails-example/

Update 3

I've learned (or at least I believe I've learned) that this might be caused by Turbolinks rendering a preview from cache and components in there being mounted in that preview.

Otherwise, during standard navigation (via Application Visits), Turbolinks will immediately restore the page from cache and display it as a preview while simultaneously loading a fresh copy from the network. This gives the illusion of instantaneous page loads for frequently accessed locations.
https://github.com/turbolinks/turbolinks#understanding-caching

rmosolgo · 2017-04-11T17:46:19Z

Thanks for all the details on this @sevos, I'll include your patch in 2.0.0 via #690

JohnSmall · 2017-04-16T07:19:29Z

Ah, I have this problem and found this thread. It's closed, but what do I do to fix the problem? Use the Github version of the gem rather than the published version?

And what else do I need to do? I see loads of comments about changing Turbolinks caching, but I've no idea what I have to do.

My problem is very simple. I've created a Bootstrap tabbed page using React.DOM.etc any time I click on the tabs I get this warning and the tabbed panes don't update.

rmosolgo · 2017-04-16T13:02:08Z

the 2.0.2 version on Rubygems includes this fix, so you should try upgrading to that version.

If you're still having a problem, could you share your Rails view code for the page with tabs and one of the React components rendered there?

zacharyw · 2017-05-29T21:39:48Z

@rmosolgo I'm getting this error in react-rails 2.2.0. An example is this project: https://github.com/zacharyw/rcg/tree/enchancement/es6

If I load up the conversations index page (the root route by default), everything is fine. If I click into a conversation, that works fine. If I click back to the home page, that's when I see the following error:

Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React.

After that point it happens any time I click on a conversation or click back to the index page. My components are in app/javascript/components, the views in question are app/views/conversations/index.html.erb and app/views/conversations/show.html.erb.

In the index view I am rendering the conversation_list component, in the show view I am rendering the message_list component.

Also, I believe this only started occurring when I upgraded from webpacker gem 1.2 to webpacker 2.0.

niltonvasques · 2017-12-18T08:08:07Z

Also happens with me, react-rails (2.4.3).

PQALAB · 2019-02-14T21:55:01Z

I'm seeing this issue with react-rails 2.4.7, and webpacker 3.5.

szyablitsky mentioned this issue Jan 12, 2017

Components aren't unmounted when Turbolinks cache is disabled shakacode/react_on_rails#629

Closed

sevos mentioned this issue Feb 25, 2017

Support component mounting and unmounting in Turbolinks and PJAX renchap/webpacker-react#14

Merged

rmosolgo modified the milestone: 2.0.0 Apr 11, 2017

rmosolgo mentioned this issue Apr 11, 2017

fix(turbolinks) use good turbolinks events #690

Merged

rmosolgo closed this as completed Apr 11, 2017

zacharyw mentioned this issue Jun 2, 2017

Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React. #732

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

turbolinks:before-render for unmounting causes warnings #607

turbolinks:before-render for unmounting causes warnings #607

thebadmonkeydev commented Sep 30, 2016

benoitongit commented Dec 12, 2016 •

edited

Loading

rmosolgo commented Dec 12, 2016

benoitongit commented Dec 12, 2016

benoitongit commented Dec 13, 2016

rmosolgo commented Dec 13, 2016

benoitongit commented Dec 13, 2016

thebadmonkeydev commented Dec 13, 2016

rmosolgo commented Dec 13, 2016

benoitongit commented Dec 13, 2016

benoitongit commented Dec 14, 2016

perezperret commented Feb 1, 2017

wdiechmann commented Feb 3, 2017

szyablitsky commented Feb 3, 2017

szyablitsky commented Feb 4, 2017

szyablitsky commented Feb 6, 2017 •

edited

Loading

renchap commented Feb 26, 2017

borisrorsvort commented Mar 14, 2017

szyablitsky commented Mar 15, 2017 •

edited

Loading

borisrorsvort commented Mar 15, 2017 •

edited

Loading

szyablitsky commented Mar 15, 2017

szyablitsky commented Mar 15, 2017

sevos commented Mar 15, 2017

szyablitsky commented Mar 15, 2017

sevos commented Mar 15, 2017 •

edited

Loading

borisrorsvort commented Mar 15, 2017

leonelgalan commented Apr 10, 2017 •

edited

Loading

rmosolgo commented Apr 11, 2017

JohnSmall commented Apr 16, 2017

rmosolgo commented Apr 16, 2017

zacharyw commented May 29, 2017 •

edited

Loading

niltonvasques commented Dec 18, 2017

PQALAB commented Feb 14, 2019

turbolinks:before-render for unmounting causes warnings #607

turbolinks:before-render for unmounting causes warnings #607

Comments

thebadmonkeydev commented Sep 30, 2016

benoitongit commented Dec 12, 2016 • edited Loading

rmosolgo commented Dec 12, 2016

benoitongit commented Dec 12, 2016

benoitongit commented Dec 13, 2016

rmosolgo commented Dec 13, 2016

benoitongit commented Dec 13, 2016

thebadmonkeydev commented Dec 13, 2016

rmosolgo commented Dec 13, 2016

benoitongit commented Dec 13, 2016

benoitongit commented Dec 14, 2016

perezperret commented Feb 1, 2017

wdiechmann commented Feb 3, 2017

szyablitsky commented Feb 3, 2017

szyablitsky commented Feb 4, 2017

szyablitsky commented Feb 6, 2017 • edited Loading

renchap commented Feb 26, 2017

borisrorsvort commented Mar 14, 2017

szyablitsky commented Mar 15, 2017 • edited Loading

borisrorsvort commented Mar 15, 2017 • edited Loading

szyablitsky commented Mar 15, 2017

szyablitsky commented Mar 15, 2017

sevos commented Mar 15, 2017

szyablitsky commented Mar 15, 2017

sevos commented Mar 15, 2017 • edited Loading

borisrorsvort commented Mar 15, 2017

leonelgalan commented Apr 10, 2017 • edited Loading

Update 1

Update 2

Update 3

rmosolgo commented Apr 11, 2017

JohnSmall commented Apr 16, 2017

rmosolgo commented Apr 16, 2017

zacharyw commented May 29, 2017 • edited Loading

niltonvasques commented Dec 18, 2017

PQALAB commented Feb 14, 2019

benoitongit commented Dec 12, 2016 •

edited

Loading

szyablitsky commented Feb 6, 2017 •

edited

Loading

szyablitsky commented Mar 15, 2017 •

edited

Loading

borisrorsvort commented Mar 15, 2017 •

edited

Loading

sevos commented Mar 15, 2017 •

edited

Loading

leonelgalan commented Apr 10, 2017 •

edited

Loading

zacharyw commented May 29, 2017 •

edited

Loading