Skip to content

shouldLeaveMarkup flag for unmountComponentAtNode#8928

Closed
szyablitsky wants to merge 5 commits into
react:masterfrom
szyablitsky:master
Closed

shouldLeaveMarkup flag for unmountComponentAtNode#8928
szyablitsky wants to merge 5 commits into
react:masterfrom
szyablitsky:master

Conversation

@szyablitsky

Copy link
Copy Markdown

This can help to resolve the issue when using React with turbolinks and libraries like react-rails or react-on-rails. Turbolinks turns any conventional web application into pseudo-SPA.

When you follow a link, Turbolinks automatically fetches the page, swaps in its , and merges its , all without incurring the cost of a full page load.

Both react-rails and react-on-rails uses turbolinks events to know when to mount and unmount React components on page.

They can use two events for unmounting:

First is turbolinks:before-render. It happens just before turbolinks swaps the page body. But using this event leads to warnings in console.

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

Second is turbolinks:before-visit. It happens before turbolinks fetch next page body from server. But unmounting components at this point and removing rendered markup from DOM causes visual changes on page which is bad for UX. By adding sholdLeaveMarkup flag for unmountComponentAtNode we can free the memory used by component but leave markup in DOM while turbolinks loads new page.

I tested these changes on my fork, and they resolve the issue with "flickering" components on turbolinks navigation.

@kodram

kodram commented Feb 4, 2017

Copy link
Copy Markdown

👍

@aweary aweary requested a review from gaearon February 4, 2017 17:44
@gaearon

gaearon commented Feb 4, 2017

Copy link
Copy Markdown
Collaborator

I don’t fully understand what you’re trying to do since I don’t know a lot about TurboLinks work.
Can you describe how it works currently in detail, step by step?
In particular I’m interested why you get this warning:

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

Generally the rule of thumb is that you can’t mutate DOM nodes managed by React, and you can’t mount React into an existing DOM tree unless it was server-rendered by React. Do TurboLinks work with these constraints?

@szyablitsky

szyablitsky commented Feb 6, 2017

Copy link
Copy Markdown
Author

The process in details (as I understand it):

Turbolinks only provide an infrastructure for seamless navigation between pages of a web-application. It intercepts any clicks on <a> tags and loads requested page via ajax. When page is loaded, it replaces <body> оf current page and merges any new elements into <head>.

All interactions with React is done by ReactOnRails (or ReactRails) front-end library. When turbolinks loads a page (for the first time or by fetching new page, user navigated to), it fires turbolinks:load event which is handled by ReactOnRails hook. ReactOnRails finds on new page every element marked by specific data- attribute (it can be empty node or server rendered React Component) and renders corresponding React Component in it.

When user clicks a link, Turbolinks fires turbolinks:before-visit event and fetches new page via ajax. When it gets new page it fires tirbolinks:before-render event, then replaces the body with the new, which was fetched form server.

ReactOnRails currently handles turbolinks:before-visit event. For every rendered component it calls ReactDOM.unmountComponentAtNode. The markup for React components is gone and user sees partially empty page while turbolinks fetches new page from server.

Before ReactOnRails used turbolinks:before-render event for unmounting components. But many users of ReactOnRails (including me) got

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

I currently do not understand why this warning happens and not sure is it permanent and reproducible. I will investigate it further. But in meantime I'd like to understand what is wrong with unmounting component by another copy of React. What problems can it emerge.

@gaearon

gaearon commented Feb 6, 2017

Copy link
Copy Markdown
Collaborator

I currently do not understand why this warning happens and not sure is it permanent and reproducible. I will investigate it further.

I think this is the actionable item here. 👍

But in meantime I'd like to understand what is wrong with unmounting component by another copy of React. What problems can it emerge.

Different versions of React are likely to have different internal APIs. If one version tries to tear down a tree created by another version, it is likely it will expect different internal properties and methods to exist, and can crash.

@gaearon

gaearon commented Feb 6, 2017

Copy link
Copy Markdown
Collaborator

I’ll close since we’re unlikely to add this to the public API for now.

The real solution indeed seems to be to investigate why that warning fires. For example it might mean client-side React is being replaced in <head>, and a new copy of React is being used to tear down the tree created by the old copy.

The most straightforward solution to me would be to unmount React components just before the page is being swapped but in the same tick. This way user wouldn’t see a flash of broken content, and you would still be using the old version of React before the new one is loaded. I hope this makes sense!

@gaearon gaearon closed this Feb 6, 2017
@szyablitsky

Copy link
Copy Markdown
Author

Ok, Dan. Thank you for clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants