Conversation
📝 WalkthroughWalkthroughFive extension files in the post-renderer feature are hardened with defensive DOM handling. Each adds try/catch blocks, validates element connectivity before DOM operations, logs warnings for disconnected elements, and performs safety checks before replacing nodes to prevent crashes from detached elements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/features/post-renderer/components/extensions/twitter-extension.tsx (1)
26-52: Avoid clearing the link before the final connectivity check.
element.innerHTML = ""runs before the finalisConnectedguard; if the node detaches in between, the original link can be emptied with no replacement. Move the DOM mutation inside the final guard and usereplaceChildrento swap in the container safely.🐛 Proposed fix
- element.classList.add("ecency-renderer-twitter-extension"); - - element.innerHTML = ""; - - // Final safety check before appending - if (element.isConnected && element.parentNode) { - element.appendChild(container); + // Final safety check before appending + if (element.isConnected && element.parentNode) { + element.classList.add("ecency-renderer-twitter-extension"); + element.replaceChildren(container); const root = createRoot(container); root.render(<ComponentInstance id={tweetId} />); }
🧹 Nitpick comments (1)
apps/web/src/features/post-renderer/components/extensions/tag-link-extension.tsx (1)
24-47: Consider unmounting React roots on cleanup.
createRootis used for each tag, but there’s no cleanup when the extension unmounts. Track roots and unmount in theuseEffectcleanup to avoid lingering trees.♻️ Proposed refactor
useEffect(() => { + const roots: Array<ReturnType<typeof createRoot>> = []; const elements = Array.from( containerRef.current?.querySelectorAll<HTMLElement>( ".markdown-view:not(.markdown-view-pure) .markdown-tag-link" ) ?? [] ); elements.forEach((element) => { try { @@ - const root = createRoot(container); + const root = createRoot(container); + roots.push(root); root.render(<TagLinkRenderer tag={element.innerText} />); @@ } catch (error) { console.warn("Error enhancing tag link element:", error); } }); + + return () => { + roots.forEach((root) => root.unmount()); + }; }, []);
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.