parentNode handling

[feruzm]

Summary by CodeRabbit

• Bug Fixes
  • Enhanced auto-update functionality with improved error handling and defensive programming practices.
  • Increased stability and reliability by reducing errors that occur during update operations.
  • Better handling of edge cases and race conditions in the update lifecycle.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The safeAutoUpdate function is enhanced with defensive error handling and lifecycle guards. Changes include connectivity pre-checks, try/catch wrapping around initialization, nullable state management, MutationObserver callback protection, and safe cleanup with error logging instead of throwing exceptions.

Changes

Cohort / File(s)	Summary
DOM lifecycle error handling apps/web/src/features/ui/util/safe-auto-update.ts	Added pre-connection checks for DOM elements; wrapped autoUpdate initialization and callback in try/catch blocks; introduced nullable cleanup and observer state; enhanced observer disconnect and cleanup safety; error logging via console.debug instead of throwing.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Poem

🐰 A rabbit's ode to defensive walls,
Where errors caught before they fall,
With guards on guards, both tried and true,
Our DOM dance now safer through and through. 🛡️

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'parentNode handling' is vague and does not clearly convey the main purpose of the changes, which involve robust DOM lifecycle management with defensive checks, error handling, and race-condition resilience.	Revise the title to be more descriptive of the actual changes, such as 'Add defensive error handling and connectivity checks to safeAutoUpdate' or 'Improve safeAutoUpdate robustness with try/catch guards and cleanup safety'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

apps/web/src/features/ui/util/safe-auto-update.ts (1)

50-53: Consider the performance implications of broad MutationObserver scope.

Observing the entire document with { childList: true, subtree: true } triggers the callback on every DOM mutation. While the callback is lightweight, this could add overhead on pages with frequent DOM updates.

An alternative approach would be to observe only the ancestors of the floating element up to the body, which would reduce the callback frequency while still detecting removal.

🔎 Alternative approach using ancestor observation

// Instead of observing the entire document, observe ancestor chainconstobserveAncestors=(element: Element): MutationObserver=>{constobserver=newMutationObserver(()=>{if(!element.isConnected){cleanup?.();observer.disconnect();cleanup=null;}});letcurrent: Node|null=element.parentNode;while(current&&current!==document){observer.observe(current,{childList: true});current=current.parentNode;}returnobserver;};

That said, the current implementation is correct and acceptable if performance isn't a concern for your use case.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 01d305d and 7646b53.

📒 Files selected for processing (1)

• apps/web/src/features/ui/util/safe-auto-update.ts

🔇 Additional comments (4)

apps/web/src/features/ui/util/safe-auto-update.ts (4)

3-15: LGTM!

The JSDoc clearly documents the purpose and the existing null checks correctly return a no-op function early when elements are missing.

---

17-23: Good defensive check for virtual elements.

The instanceof Node check correctly handles virtual reference elements (non-DOM objects) that don't have isConnected. This appropriately treats them as "always connected" since they have no DOM parentage.

---

33-48: LGTM!

The MutationObserver callback correctly:

• Uses consistent instanceof Node check for reference
• Safely cleans up and nullifies references to prevent double-cleanup
• Logs errors without throwing to avoid breaking the application

---

61-69: LGTM!

The cleanup function correctly:

• Uses optional chaining to handle cases where cleanup already occurred via the MutationObserver
• Wraps in try-catch to prevent cleanup errors from propagating
• Logs errors for debugging without breaking the application

The order (disconnect before cleanup) is intentional and safe—stopping observation before stopping autoUpdate prevents potential race conditions where the observer callback fires during cleanup.