Skip to content

UseOnRead element cleanup#710

Merged
kentcdodds merged 1 commit intomainfrom
cursor/useonread-element-cleanup-12a3
Feb 26, 2026
Merged

UseOnRead element cleanup#710
kentcdodds merged 1 commit intomainfrom
cursor/useonread-element-cleanup-12a3

Conversation

@kentcdodds
Copy link
Owner

@kentcdodds kentcdodds commented Feb 26, 2026

Make the useOnRead hook's cleanup idempotent to prevent a TypeError when an IntersectionObserver callback attempts to remove an already detached element.

Fixes KCD-NODE-W0


Open in Web Open in Cursor 


Note

Low Risk
Low risk: small, localized change to client-side observer/DOM cleanup to avoid double-removal errors; behavior should be unchanged aside from preventing a runtime exception.

Overview
Fixes useOnRead in app/routes/blog_/$slug.tsx to make removal of its visibilityEl idempotent.

The hook now tracks the element as nullable and centralizes removal via removeVisibilityEl(), guarding both the IntersectionObserver callback and effect cleanup against attempting to remove an already-detached element.

Written by Cursor Bugbot for commit c594160. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced stability of blog post visibility tracking by improving internal reference management and ensuring complete resource cleanup during component lifecycle transitions. This prevents potential issues with element handling.

@cursor
Copy link

cursor bot commented Feb 26, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@coderabbitai
Copy link

coderabbitai bot commented Feb 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11b80b6 and c594160.

📒 Files selected for processing (1)
  • app/routes/blog_/$slug.tsx

📝 Walkthrough

Walkthrough

Refactored the blog slug route to improve visibility element cleanup handling. Introduced a removeVisibilityEl helper function to safely remove the visibility element, replacing direct .remove() calls. Updated the IntersectionObserver to use a local reference and guard element operations, ensuring proper cleanup and preventing null/garbage references.

Changes

Cohort / File(s) Summary
Visibility Element Cleanup Refactor
app/routes/blog_/$slug.tsx
Introduced removeVisibilityEl safe removal helper, replaced direct element removal calls with guarded helper invocations, and refactored observer cleanup paths to prevent null/garbage references.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hoppity-hop, the cleanup's tight,
No more null refs in the night,
A safety helper, swift and true,
Keeps the DOM fresh and spry—who knew!
Memory flows like carrots flow,
Through the observer's gentle glow. 🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'UseOnRead element cleanup' is directly related to the main change: fixing element cleanup in the useOnRead hook by introducing safe removal helpers and making cleanup idempotent.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cursor/useonread-element-cleanup-12a3

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@kentcdodds kentcdodds merged commit 733a15a into main Feb 26, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants