Components not cleaned up with turbo links navigation, part 2.
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
- Issue type
- Bug
- Clarity
- Mostly clear
- Activity status
- Stale
- Tech stack
- javascript, rails, react, ruby
Research direction
Reproduce the behavior with ReactUJS 2.6.2, Rails 7.0.2, and Turbolinks navigation using the listed componentWillUnmount steps. Read PR1135 and the ReactUJS cleanup behavior; done means components unmount on navigation so intervals and in-progress work can be cleaned up without breaking scroll-position restoration.
Written by the indexing model from the issue text.
Description
(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 ..?
- Dominant language
- JavaScript
- Stars
- 6.8k
- Forks
- 739
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from reactjs/react-rails
-
Documentation
Difficulty 4/5 3-5 days Newbie friendliness 45/100
reactjs/react-rails#1380 · 1 comment ·
-
Difficulty 4/5 3-5 days Newbie friendliness 38/100
reactjs/react-rails#1355 · 1 comment · 1 reaction ·
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
reactjs/react-rails#1351 · 4 comments · 10 reactions ·
-
discussion
Difficulty 4/5 3-5 days Newbie friendliness 35/100
reactjs/react-rails#1341 · 2 comments ·
-
Difficulty 4/5 3-5 days Newbie friendliness 25/100
reactjs/react-rails#1338 · 3 comments ·
All issues in reactjs/react-rails
Similar issues
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
avniproject/avni-client#2135 ·
-
automated broken-link
Difficulty 1/5 Under an hour Newbie friendliness 85/100
-
agent/security hive/hosted-available-lke648397-260827-5n31 security
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 70/100
babalae/bettergi-scripts-list#3674 ·
-
A-Release-Notes C-Editing D-Modest S-Ready-For-Implementation
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
bevyengine/bevy-website#2595 ·