Components not cleaned up with turbo links navigation, part 2.

[jdelStrother]

(I'd originally mentioned this in https://github.com/reactjs/react-rails/issues/1028, but since that's closed I wanted to open an issue for it to get some extra visibility)

Steps to reproduce

• Write a component that performs cleanup in componentWillUnmount
• visit a page containing that component
• navigate away with turbolinks

Expected behavior

componentWillUnmount is called

Actual behavior

componentWillUnmount is never called

System configuration

Sprockets or Webpacker version: 6.4.1 React-Rails version: - Rect_UJS version: 2.6.2 Rails version: 7.0.2 Ruby version: 3.1

---

What's the expected way of cleaning up react components on leaving the page?

Some of our components use setInterval to do something every x seconds, which call clearInterval in componentWillUnmount. Others might load and/or decode a large file, which gets aborted in componentWillUnmount.

I could possibly clean these up by having each component listen for turbolinks cleanup events, but having those components have to know that they're living in a turbolinks+react_ujs stack seems like unnecessary coupling.

I think this used to work, but PR1135 removed cleanup in favour of fixing the scroll-position restoration. That seems like something that should have been fixed upstream in Turbolinks rather than removing component-unmounts from ReactUJS ..?

[toreyhickman]

Just ran into this as well when upgrading from Version 2.6.1 to Version 2.6.2. In our case we have components on some pages that add event listeners when mounted and then remove the event listeners when unmounted. Without the components being unmounted, the listeners are never removed, rather they are added again and again as users navigate through the site.

To correct this, I undid the changes made in #1135 by adding ...

ReactRailsUJS.handleEvent('turbolinks:before-render', ReactRailsUJS.handleUnmount)

... to app/javascript/packs/application.js.

Is there a better way to handle this?

[lcmen]

@toreyhickman does this work for you with React 18?

I'm getting a following warning on the console and components are not being unloaded:

react_devtools_backend.js:4026 Warning: You are calling ReactDOM.unmountComponentAtNode() on a container that was previously passed to ReactDOMClient.createRoot(). This is not supported. Did you mean to call root.unmount()?
react_devtools_backend.js:4026 Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by React and is not a top-level container. Instead, have the parent component update its state and rerender in order to remove this component.

[toreyhickman]

@toreyhickman does this work for you with React 18?

I'm getting a following warning on the console and components are not being unloaded:

react_devtools_backend.js:4026 Warning: You are calling ReactDOM.unmountComponentAtNode() on a container that was previously passed to ReactDOMClient.createRoot(). This is not supported. Did you mean to call root.unmount()?
react_devtools_backend.js:4026 Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by React and is not a top-level container. Instead, have the parent component update its state and rerender in order to remove this component.

@lcmen: The project I'm on is using React 17. I don't know if something in Version 18 would lead to that warning, but I'm not seeing the warning with Version 17.

[lcmen]

@toreyhickman I believe React 17 is not affected by this issue just 18 is.

[kylemellander]

This is definitely an issue. While manually adding the event listener to unmount the components "works" it is not ideal for multiple reasons.

1. You have scenarios like this
2. Unchanged or data-turbolinks-permanent components unmount unnecessarily

I wonder if there should be some logic that compares the new DOM to the old one to see if the element exists in the new DOM and unmount if it doesn't.

[justin808]

I'd be happy to consider a PR to fix this. @toreyhickman @lcmen @kylemellander. Let me know!