test/zIndex/DynamicZIndex.spec.tsx (2)

7-7: vi.useFakeTimers() should be inside beforeEach with corresponding afterEach cleanup.

Calling vi.useFakeTimers() at the describe-level top scope means it runs once when the module is loaded. The conventional pattern is to set up and tear down fake timers per test to avoid leaking timer state between tests:

Proposed fix

 describe('Dynamic zIndex updates', () => {
-  vi.useFakeTimers();
+  beforeEach(() => {
+    vi.useFakeTimers();
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });

---

36-39: Consider adding vi.runOnlyPendingTimers() inside the waitFor or before it.

With fake timers, waitFor may not advance time on its own. If portal registration relies on batched Redux updates that need a timer tick, this waitFor could time out. Wrapping a vi.runOnlyPendingTimers() call before the assertions (or using advanceTimersByTime) ensures the async portal registration completes.

src/zIndex/ZIndexLayer.tsx (2)

131-137: Verify the unmount effect captures a live reference to the Set.

Line 132 captures registeredZIndexesRef.current into registered. Because the Set is mutated in place (never replaced with a new Set), this captured reference stays current through the component's lifetime, so the cleanup correctly unregisters all active entries at unmount. This works, but it's subtly relying on the fact that registeredZIndexesRef.current is never reassigned to a new Set. If a future refactor were to replace the Set (e.g., registeredZIndexesRef.current = new Set()), this cleanup would operate on a stale reference. Consider adding a brief comment noting this assumption.

---

82-83: Minor: consider documenting that registeredZIndexesRef must never be replaced.

The entire cleanup and lifecycle logic depends on registeredZIndexesRef.current always being the same Set instance (mutated via .add/.delete/.clear). This invariant is key to correctness — the unmount effect captures this reference once. A brief inline comment would protect against accidental breakage.

src/zIndex/ZIndexLayer.tsx (2)

101-136: Lifecycle effect logic is sound; one minor concern with Set mutation during iteration.

Lines 129–133 delete entries from registeredZIndexesRef.current while iterating it with forEach. Per the ES spec this is safe (already-visited and currently-visited entries can be deleted without affecting remaining iteration), but it's a pattern that surprises readers. Consider collecting stale keys first and deleting in a second pass for clarity—though this is purely a readability nit.

Optional: two-pass cleanup for readability

     if (portalElement) {
       lastPortalElementRef.current = portalElement;
       const registered = registeredZIndexesRef.current;
-      registered.forEach(z => {
-        if (z !== zIndex) {
-          dispatch(unregisterZIndexPortal({ zIndex: z }));
-          registered.delete(z);
-        }
-      });
+      const stale = [...registered].filter(z => z !== zIndex);
+      stale.forEach(z => {
+        dispatch(unregisterZIndexPortal({ zIndex: z }));
+        registered.delete(z);
+      });
     }

---

138-147: Unmount cleanup relies on dispatch stability—verify this holds.

The effect captures registeredZIndexesRef.current on setup (Line 140) and the cleanup closure references dispatch. Since Redux's useDispatch returns a stable reference this works correctly, but if a future refactor wraps dispatch the [dispatch] dep would cause the cleanup to fire mid-lifecycle, unregistering everything. A brief inline comment noting the stability assumption would help future maintainers.