From 458d11e2629ff138a8f80e1dd248c0ce75f82c1e Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 4 Feb 2026 12:17:31 -0700 Subject: [PATCH 1/9] use flushSync and add console.logs --- .../flyout/manager/flyout_managed.tsx | 66 ++++++++++++++++--- .../src/components/flyout/manager/reducer.ts | 45 +++++++++++++ packages/eui/src/components/portal/portal.tsx | 24 +++++++ 3 files changed, 125 insertions(+), 10 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx index 25d069c13fdb..b60223389317 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.tsx @@ -7,6 +7,7 @@ */ import React, { useEffect, useMemo, useRef, useState, forwardRef } from 'react'; +import { flushSync } from 'react-dom'; import { useCombinedRefs, useEuiMemoizedStyles } from '../../../services'; import { useEuiI18n } from '../../i18n'; import { useResizeObserver } from '../../observer/resize_observer'; @@ -170,13 +171,38 @@ export const EuiManagedFlyout = forwardRef( // Register with flyout manager context when open, remove when closed useEffect(() => { + console.log('[EUI MANAGED FLYOUT] Primary effect: Registering flyout:', { + flyoutId, + level, + timestamp: Date.now(), + }); addFlyout(flyoutId, title!, level, size as string); return () => { + console.log('[EUI MANAGED FLYOUT] Primary cleanup effect triggered:', { + flyoutId, + flyoutExistsInManager, + level, + timestamp: Date.now(), + }); // Only call closeFlyout if it wasn't already called via onClose // This prevents duplicate removal when using Escape/X button if (flyoutExistsInManager) { + console.log( + '[EUI MANAGED FLYOUT] Primary cleanup: Calling closeFlyout' + ); closeFlyout(flyoutId); + console.log( + '[EUI MANAGED FLYOUT] Primary cleanup: closeFlyout completed:', + { + flyoutId, + timestamp: Date.now(), + } + ); + } else { + console.log( + '[EUI MANAGED FLYOUT] Primary cleanup: Skipping closeFlyout (already removed)' + ); } // Reset navigation tracking when explicitly closed via isOpen=false @@ -224,31 +250,51 @@ export const EuiManagedFlyout = forwardRef( } }, [currentSession, flyoutId, level]); - useEffect(() => { - return () => { - // Only remove from manager on component unmount if still registered - // This cleanup serves as a final safety net for edge cases - if (flyoutExistsInManager) { - closeFlyout(flyoutId); - } - }; - }, [closeFlyout, flyoutId, flyoutExistsInManager]); + // REMOVED: Secondary cleanup effect was causing duplicate closeFlyout calls + // which contributed to race conditions during portal cleanup. + // The primary cleanup effect (lines 172-211) is sufficient. + // See: FLYOUT_BUG_ANALYSIS.md for details on the race condition fix. // Track width changes for flyouts const { width } = useResizeObserver(isActive ? flyoutRef : null, 'width'); // Pass the stabilized onClose callback to the flyout menu context const onClose = (e?: EuiFlyoutCloseEvent) => { + console.log('[EUI MANAGED FLYOUT] onClose START:', { + flyoutId, + level, + eventType: e?.type || 'no-event', + flyoutExistsInManager, + timestamp: Date.now(), + }); + // CRITICAL: Update manager state FIRST before allowing React to unmount // This prevents race conditions during portal → inline DOM transitions // and ensures cascade close logic runs before DOM cleanup begins - closeFlyout(flyoutId); + // Using flushSync to force synchronous state update completion + console.log('[EUI MANAGED FLYOUT] onClose: About to call closeFlyout with flushSync'); + flushSync(() => { + closeFlyout(flyoutId); + }); + console.log('[EUI MANAGED FLYOUT] onClose: closeFlyout completed (flushSync):', { + flyoutId, + timestamp: Date.now(), + }); // trigger parent callback, unmounts the component + console.log('[EUI MANAGED FLYOUT] onClose: About to call parent callback:', { + flyoutId, + hasCallback: !!onCloseCallbackRef.current, + timestamp: Date.now(), + }); if (onCloseCallbackRef.current) { const event = e || new MouseEvent('click'); onCloseCallbackRef.current(event); } + console.log('[EUI MANAGED FLYOUT] onClose END:', { + flyoutId, + timestamp: Date.now(), + }); }; // Update width in manager state when it changes diff --git a/packages/eui/src/components/flyout/manager/reducer.ts b/packages/eui/src/components/flyout/manager/reducer.ts index 1324e12dd696..3e58c86af430 100644 --- a/packages/eui/src/components/flyout/manager/reducer.ts +++ b/packages/eui/src/components/flyout/manager/reducer.ts @@ -142,14 +142,32 @@ export function flyoutManagerReducer( // - When closing a `child` flyout, clear the child pointer on the most // recent session if it matches. case ACTION_CLOSE: { + console.log('[EUI REDUCER] ACTION_CLOSE START:', { + flyoutId: action.flyoutId, + currentSessionsCount: state.sessions.length, + currentFlyoutsCount: state.flyouts.length, + sessions: state.sessions.map((s) => ({ + mainId: s.mainFlyoutId, + childId: s.childFlyoutId, + title: s.title, + })), + timestamp: Date.now(), + }); + const removedFlyout = state.flyouts.find( (f) => f.flyoutId === action.flyoutId ); if (!removedFlyout) { + console.log('[EUI REDUCER] ACTION_CLOSE: Flyout not found, returning state unchanged'); return state; } + console.log('[EUI REDUCER] ACTION_CLOSE: Found flyout to remove:', { + flyoutId: action.flyoutId, + level: removedFlyout.level, + }); + if (removedFlyout.level === LEVEL_MAIN) { // Find the session that contains this main flyout const sessionToRemove = state.sessions.find( @@ -157,6 +175,12 @@ export function flyoutManagerReducer( ); if (sessionToRemove) { + console.log('[EUI REDUCER] ACTION_CLOSE: Removing main flyout session:', { + flyoutId: action.flyoutId, + sessionToRemove, + timestamp: Date.now(), + }); + // Remove all flyouts associated with this session (main + child) const flyoutsToRemove = new Set([action.flyoutId]); if (sessionToRemove.childFlyoutId) { @@ -178,6 +202,18 @@ export function flyoutManagerReducer( newCurrentZIndex = 0; } + console.log('[EUI REDUCER] ACTION_CLOSE: Returning new state after main flyout removal:', { + flyoutId: action.flyoutId, + newSessionsCount: newSessions.length, + newFlyoutsCount: newFlyouts.length, + remainingSessions: newSessions.map((s) => ({ + mainId: s.mainFlyoutId, + childId: s.childFlyoutId, + title: s.title, + })), + timestamp: Date.now(), + }); + return { ...state, sessions: newSessions, @@ -188,11 +224,13 @@ export function flyoutManagerReducer( } // Handle child flyout closing (existing logic) + console.log('[EUI REDUCER] ACTION_CLOSE: Handling child flyout closure'); const newFlyouts = state.flyouts.filter( (f) => f.flyoutId !== action.flyoutId ); if (state.sessions.length === 0) { + console.log('[EUI REDUCER] ACTION_CLOSE: No sessions, returning state with updated flyouts'); return { ...state, flyouts: newFlyouts }; } @@ -208,6 +246,13 @@ export function flyoutManagerReducer( }; } + console.log('[EUI REDUCER] ACTION_CLOSE: Returning new state after child flyout removal:', { + flyoutId: action.flyoutId, + sessionsCount: updatedSessions.length, + flyoutsCount: newFlyouts.length, + timestamp: Date.now(), + }); + return { ...state, sessions: updatedSessions, flyouts: newFlyouts }; } diff --git a/packages/eui/src/components/portal/portal.tsx b/packages/eui/src/components/portal/portal.tsx index f4f24ba9d214..37fcc42e1362 100644 --- a/packages/eui/src/components/portal/portal.tsx +++ b/packages/eui/src/components/portal/portal.tsx @@ -81,6 +81,13 @@ export class EuiPortalClass extends Component { sibling.insertAdjacentElement(insertPositions[position], portalNode); } + console.log('[EUI PORTAL] componentDidMount:', { + hasPortalNode: !!portalNode, + hasParent: !!portalNode.parentNode, + inDocument: document.body.contains(portalNode), + timestamp: Date.now(), + }); + this.setThemeColor(portalNode); this.updatePortalRef(portalNode); @@ -93,8 +100,25 @@ export class EuiPortalClass extends Component { componentWillUnmount() { const { portalNode } = this.state; + console.log('[EUI PORTAL] componentWillUnmount START:', { + hasPortalNode: !!portalNode, + hasParent: !!(portalNode?.parentNode), + inDocument: portalNode ? document.body.contains(portalNode) : false, + timestamp: Date.now(), + }); + if (portalNode?.parentNode) { + console.log('[EUI PORTAL] About to removeChild:', { + parentNode: portalNode.parentNode, + inDocument: document.body.contains(portalNode), + timestamp: Date.now(), + }); portalNode.parentNode.removeChild(portalNode); + console.log('[EUI PORTAL] removeChild completed:', { + timestamp: Date.now(), + }); + } else { + console.log('[EUI PORTAL] Skipping removeChild (no parent node)'); } this.updatePortalRef(null); } From 52f551659def47a8e8e62be27d2f6015acc1293e Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 4 Feb 2026 13:42:35 -0700 Subject: [PATCH 2/9] Remove `flyoutExistsInManager` from dependency array --- .../flyout/manager/flyout_managed.tsx | 61 ++++++++++++------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx index b60223389317..a060ab0adfda 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.tsx @@ -6,7 +6,14 @@ * Side Public License, v 1. */ -import React, { useEffect, useMemo, useRef, useState, forwardRef } from 'react'; +import React, { + useEffect, + useLayoutEffect, + useMemo, + useRef, + useState, + forwardRef, +} from 'react'; import { flushSync } from 'react-dom'; import { useCombinedRefs, useEuiMemoizedStyles } from '../../../services'; import { useEuiI18n } from '../../i18n'; @@ -169,8 +176,14 @@ export const EuiManagedFlyout = forwardRef( // Track if flyout was ever registered to avoid false positives on initial mount const wasRegisteredRef = useRef(false); + // Track flyoutExistsInManager in a ref to avoid dependency loop + // The cleanup function needs the current value but shouldn't cause re-runs + const flyoutExistsInManagerRef = useRef(flyoutExistsInManager); + flyoutExistsInManagerRef.current = flyoutExistsInManager; + // Register with flyout manager context when open, remove when closed - useEffect(() => { + // Using useLayoutEffect to run synchronously before DOM updates + useLayoutEffect(() => { console.log('[EUI MANAGED FLYOUT] Primary effect: Registering flyout:', { flyoutId, level, @@ -181,13 +194,13 @@ export const EuiManagedFlyout = forwardRef( return () => { console.log('[EUI MANAGED FLYOUT] Primary cleanup effect triggered:', { flyoutId, - flyoutExistsInManager, + flyoutExistsInManager: flyoutExistsInManagerRef.current, level, timestamp: Date.now(), }); // Only call closeFlyout if it wasn't already called via onClose // This prevents duplicate removal when using Escape/X button - if (flyoutExistsInManager) { + if (flyoutExistsInManagerRef.current) { console.log( '[EUI MANAGED FLYOUT] Primary cleanup: Calling closeFlyout' ); @@ -208,15 +221,9 @@ export const EuiManagedFlyout = forwardRef( // Reset navigation tracking when explicitly closed via isOpen=false wasRegisteredRef.current = false; }; - }, [ - flyoutId, - title, - level, - size, - addFlyout, - closeFlyout, - flyoutExistsInManager, - ]); + // CRITICAL: flyoutExistsInManager removed from dependencies + // to prevent re-registration loops during unmount + }, [flyoutId, title, level, size, addFlyout, closeFlyout]); // Detect when flyout has been removed from manager state (e.g., via Back button) // and trigger onClose callback to notify the parent component @@ -272,21 +279,29 @@ export const EuiManagedFlyout = forwardRef( // This prevents race conditions during portal → inline DOM transitions // and ensures cascade close logic runs before DOM cleanup begins // Using flushSync to force synchronous state update completion - console.log('[EUI MANAGED FLYOUT] onClose: About to call closeFlyout with flushSync'); + console.log( + '[EUI MANAGED FLYOUT] onClose: About to call closeFlyout with flushSync' + ); flushSync(() => { closeFlyout(flyoutId); }); - console.log('[EUI MANAGED FLYOUT] onClose: closeFlyout completed (flushSync):', { - flyoutId, - timestamp: Date.now(), - }); + console.log( + '[EUI MANAGED FLYOUT] onClose: closeFlyout completed (flushSync):', + { + flyoutId, + timestamp: Date.now(), + } + ); // trigger parent callback, unmounts the component - console.log('[EUI MANAGED FLYOUT] onClose: About to call parent callback:', { - flyoutId, - hasCallback: !!onCloseCallbackRef.current, - timestamp: Date.now(), - }); + console.log( + '[EUI MANAGED FLYOUT] onClose: About to call parent callback:', + { + flyoutId, + hasCallback: !!onCloseCallbackRef.current, + timestamp: Date.now(), + } + ); if (onCloseCallbackRef.current) { const event = e || new MouseEvent('click'); onCloseCallbackRef.current(event); From ad0176ceb3fd30f959cb93180e9dcf6d505b8518 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 4 Feb 2026 13:47:54 -0700 Subject: [PATCH 3/9] more debug logs --- .../flyout/manager/flyout_managed.tsx | 43 ++++++++++++++++--- .../src/components/flyout/manager/reducer.ts | 27 +++++++++++- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx index a060ab0adfda..fbe5adce777e 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.tsx @@ -179,7 +179,18 @@ export const EuiManagedFlyout = forwardRef( // Track flyoutExistsInManager in a ref to avoid dependency loop // The cleanup function needs the current value but shouldn't cause re-runs const flyoutExistsInManagerRef = useRef(flyoutExistsInManager); - flyoutExistsInManagerRef.current = flyoutExistsInManager; + + // Log when flyoutExistsInManager changes to track state transitions + useEffect(() => { + console.log('[EUI MANAGED FLYOUT] flyoutExistsInManager changed:', { + flyoutId, + level, + previousValue: flyoutExistsInManagerRef.current, + newValue: flyoutExistsInManager, + timestamp: Date.now(), + }); + flyoutExistsInManagerRef.current = flyoutExistsInManager; + }, [flyoutExistsInManager, flyoutId, level]); // Register with flyout manager context when open, remove when closed // Using useLayoutEffect to run synchronously before DOM updates @@ -187,6 +198,10 @@ export const EuiManagedFlyout = forwardRef( console.log('[EUI MANAGED FLYOUT] Primary effect: Registering flyout:', { flyoutId, level, + flyoutExistsInManagerRef: flyoutExistsInManagerRef.current, + flyoutExistsInManager, + title, + size, timestamp: Date.now(), }); addFlyout(flyoutId, title!, level, size as string); @@ -194,15 +209,21 @@ export const EuiManagedFlyout = forwardRef( return () => { console.log('[EUI MANAGED FLYOUT] Primary cleanup effect triggered:', { flyoutId, - flyoutExistsInManager: flyoutExistsInManagerRef.current, + flyoutExistsInManagerRef: flyoutExistsInManagerRef.current, level, + wasRegistered: wasRegisteredRef.current, timestamp: Date.now(), + stackTrace: new Error().stack?.split('\n').slice(1, 4).join(' | '), }); // Only call closeFlyout if it wasn't already called via onClose // This prevents duplicate removal when using Escape/X button if (flyoutExistsInManagerRef.current) { console.log( - '[EUI MANAGED FLYOUT] Primary cleanup: Calling closeFlyout' + '[EUI MANAGED FLYOUT] Primary cleanup: Calling closeFlyout', + { + flyoutId, + timestamp: Date.now(), + } ); closeFlyout(flyoutId); console.log( @@ -214,12 +235,20 @@ export const EuiManagedFlyout = forwardRef( ); } else { console.log( - '[EUI MANAGED FLYOUT] Primary cleanup: Skipping closeFlyout (already removed)' + '[EUI MANAGED FLYOUT] Primary cleanup: Skipping closeFlyout (already removed)', + { + flyoutId, + timestamp: Date.now(), + } ); } // Reset navigation tracking when explicitly closed via isOpen=false wasRegisteredRef.current = false; + console.log('[EUI MANAGED FLYOUT] Primary cleanup complete:', { + flyoutId, + timestamp: Date.now(), + }); }; // CRITICAL: flyoutExistsInManager removed from dependencies // to prevent re-registration loops during unmount @@ -298,16 +327,20 @@ export const EuiManagedFlyout = forwardRef( '[EUI MANAGED FLYOUT] onClose: About to call parent callback:', { flyoutId, + level, hasCallback: !!onCloseCallbackRef.current, timestamp: Date.now(), } ); if (onCloseCallbackRef.current) { const event = e || new MouseEvent('click'); + console.log('[EUI MANAGED FLYOUT] onClose: Executing parent callback (triggers unmount)'); onCloseCallbackRef.current(event); + console.log('[EUI MANAGED FLYOUT] onClose: Parent callback returned'); } - console.log('[EUI MANAGED FLYOUT] onClose END:', { + console.log('[EUI MANAGED FLYOUT] onClose END (returning to React):', { flyoutId, + level, timestamp: Date.now(), }); }; diff --git a/packages/eui/src/components/flyout/manager/reducer.ts b/packages/eui/src/components/flyout/manager/reducer.ts index 3e58c86af430..c00da6a7b3e3 100644 --- a/packages/eui/src/components/flyout/manager/reducer.ts +++ b/packages/eui/src/components/flyout/manager/reducer.ts @@ -87,8 +87,21 @@ export function flyoutManagerReducer( case ACTION_ADD: { const { flyoutId, title, level, size } = action; + console.log('[EUI REDUCER] ACTION_ADD:', { + flyoutId, + level, + title, + currentSessionsCount: state.sessions.length, + currentFlyoutsCount: state.flyouts.length, + allFlyoutIds: state.flyouts.map((f) => f.flyoutId), + timestamp: Date.now(), + }); + // Ignore duplicate registrations if (state.flyouts.some((f) => f.flyoutId === flyoutId)) { + console.log('[EUI REDUCER] ACTION_ADD: Duplicate registration ignored:', { + flyoutId, + }); return state; } @@ -111,7 +124,7 @@ export function flyoutManagerReducer( zIndex: state.currentZIndex, }; - return { + const newState = { ...state, sessions: [...state.sessions, newSession], flyouts: newFlyouts, @@ -120,6 +133,16 @@ export function flyoutManagerReducer( // child flyouts on `n - 1` and the overlay mask on `n - 2`. currentZIndex: state.currentZIndex + 3, }; + + console.log('[EUI REDUCER] ACTION_ADD: Created new session for main flyout:', { + flyoutId, + newSessionsCount: newState.sessions.length, + newFlyoutsCount: newState.flyouts.length, + newSession, + timestamp: Date.now(), + }); + + return newState; } if (state.sessions.length === 0) { @@ -150,7 +173,9 @@ export function flyoutManagerReducer( mainId: s.mainFlyoutId, childId: s.childFlyoutId, title: s.title, + zIndex: s.zIndex, })), + allFlyoutIds: state.flyouts.map((f) => f.flyoutId), timestamp: Date.now(), }); From 4b5c9d12e945c06d1a8b503e6c3314dc3d938fdd Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 4 Feb 2026 14:31:31 -0700 Subject: [PATCH 4/9] remove console.log --- .../flyout/manager/flyout_managed.tsx | 88 +------------------ .../src/components/flyout/manager/reducer.ts | 72 +-------------- packages/eui/src/components/portal/portal.tsx | 23 ----- 3 files changed, 3 insertions(+), 180 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx index fbe5adce777e..d1923ce27535 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.tsx @@ -180,75 +180,25 @@ export const EuiManagedFlyout = forwardRef( // The cleanup function needs the current value but shouldn't cause re-runs const flyoutExistsInManagerRef = useRef(flyoutExistsInManager); - // Log when flyoutExistsInManager changes to track state transitions + // Update ref when flyoutExistsInManager changes useEffect(() => { - console.log('[EUI MANAGED FLYOUT] flyoutExistsInManager changed:', { - flyoutId, - level, - previousValue: flyoutExistsInManagerRef.current, - newValue: flyoutExistsInManager, - timestamp: Date.now(), - }); flyoutExistsInManagerRef.current = flyoutExistsInManager; - }, [flyoutExistsInManager, flyoutId, level]); + }, [flyoutExistsInManager]); // Register with flyout manager context when open, remove when closed // Using useLayoutEffect to run synchronously before DOM updates useLayoutEffect(() => { - console.log('[EUI MANAGED FLYOUT] Primary effect: Registering flyout:', { - flyoutId, - level, - flyoutExistsInManagerRef: flyoutExistsInManagerRef.current, - flyoutExistsInManager, - title, - size, - timestamp: Date.now(), - }); addFlyout(flyoutId, title!, level, size as string); return () => { - console.log('[EUI MANAGED FLYOUT] Primary cleanup effect triggered:', { - flyoutId, - flyoutExistsInManagerRef: flyoutExistsInManagerRef.current, - level, - wasRegistered: wasRegisteredRef.current, - timestamp: Date.now(), - stackTrace: new Error().stack?.split('\n').slice(1, 4).join(' | '), - }); // Only call closeFlyout if it wasn't already called via onClose // This prevents duplicate removal when using Escape/X button if (flyoutExistsInManagerRef.current) { - console.log( - '[EUI MANAGED FLYOUT] Primary cleanup: Calling closeFlyout', - { - flyoutId, - timestamp: Date.now(), - } - ); closeFlyout(flyoutId); - console.log( - '[EUI MANAGED FLYOUT] Primary cleanup: closeFlyout completed:', - { - flyoutId, - timestamp: Date.now(), - } - ); - } else { - console.log( - '[EUI MANAGED FLYOUT] Primary cleanup: Skipping closeFlyout (already removed)', - { - flyoutId, - timestamp: Date.now(), - } - ); } // Reset navigation tracking when explicitly closed via isOpen=false wasRegisteredRef.current = false; - console.log('[EUI MANAGED FLYOUT] Primary cleanup complete:', { - flyoutId, - timestamp: Date.now(), - }); }; // CRITICAL: flyoutExistsInManager removed from dependencies // to prevent re-registration loops during unmount @@ -296,53 +246,19 @@ export const EuiManagedFlyout = forwardRef( // Pass the stabilized onClose callback to the flyout menu context const onClose = (e?: EuiFlyoutCloseEvent) => { - console.log('[EUI MANAGED FLYOUT] onClose START:', { - flyoutId, - level, - eventType: e?.type || 'no-event', - flyoutExistsInManager, - timestamp: Date.now(), - }); - // CRITICAL: Update manager state FIRST before allowing React to unmount // This prevents race conditions during portal → inline DOM transitions // and ensures cascade close logic runs before DOM cleanup begins // Using flushSync to force synchronous state update completion - console.log( - '[EUI MANAGED FLYOUT] onClose: About to call closeFlyout with flushSync' - ); flushSync(() => { closeFlyout(flyoutId); }); - console.log( - '[EUI MANAGED FLYOUT] onClose: closeFlyout completed (flushSync):', - { - flyoutId, - timestamp: Date.now(), - } - ); // trigger parent callback, unmounts the component - console.log( - '[EUI MANAGED FLYOUT] onClose: About to call parent callback:', - { - flyoutId, - level, - hasCallback: !!onCloseCallbackRef.current, - timestamp: Date.now(), - } - ); if (onCloseCallbackRef.current) { const event = e || new MouseEvent('click'); - console.log('[EUI MANAGED FLYOUT] onClose: Executing parent callback (triggers unmount)'); onCloseCallbackRef.current(event); - console.log('[EUI MANAGED FLYOUT] onClose: Parent callback returned'); } - console.log('[EUI MANAGED FLYOUT] onClose END (returning to React):', { - flyoutId, - level, - timestamp: Date.now(), - }); }; // Update width in manager state when it changes diff --git a/packages/eui/src/components/flyout/manager/reducer.ts b/packages/eui/src/components/flyout/manager/reducer.ts index c00da6a7b3e3..1324e12dd696 100644 --- a/packages/eui/src/components/flyout/manager/reducer.ts +++ b/packages/eui/src/components/flyout/manager/reducer.ts @@ -87,21 +87,8 @@ export function flyoutManagerReducer( case ACTION_ADD: { const { flyoutId, title, level, size } = action; - console.log('[EUI REDUCER] ACTION_ADD:', { - flyoutId, - level, - title, - currentSessionsCount: state.sessions.length, - currentFlyoutsCount: state.flyouts.length, - allFlyoutIds: state.flyouts.map((f) => f.flyoutId), - timestamp: Date.now(), - }); - // Ignore duplicate registrations if (state.flyouts.some((f) => f.flyoutId === flyoutId)) { - console.log('[EUI REDUCER] ACTION_ADD: Duplicate registration ignored:', { - flyoutId, - }); return state; } @@ -124,7 +111,7 @@ export function flyoutManagerReducer( zIndex: state.currentZIndex, }; - const newState = { + return { ...state, sessions: [...state.sessions, newSession], flyouts: newFlyouts, @@ -133,16 +120,6 @@ export function flyoutManagerReducer( // child flyouts on `n - 1` and the overlay mask on `n - 2`. currentZIndex: state.currentZIndex + 3, }; - - console.log('[EUI REDUCER] ACTION_ADD: Created new session for main flyout:', { - flyoutId, - newSessionsCount: newState.sessions.length, - newFlyoutsCount: newState.flyouts.length, - newSession, - timestamp: Date.now(), - }); - - return newState; } if (state.sessions.length === 0) { @@ -165,34 +142,14 @@ export function flyoutManagerReducer( // - When closing a `child` flyout, clear the child pointer on the most // recent session if it matches. case ACTION_CLOSE: { - console.log('[EUI REDUCER] ACTION_CLOSE START:', { - flyoutId: action.flyoutId, - currentSessionsCount: state.sessions.length, - currentFlyoutsCount: state.flyouts.length, - sessions: state.sessions.map((s) => ({ - mainId: s.mainFlyoutId, - childId: s.childFlyoutId, - title: s.title, - zIndex: s.zIndex, - })), - allFlyoutIds: state.flyouts.map((f) => f.flyoutId), - timestamp: Date.now(), - }); - const removedFlyout = state.flyouts.find( (f) => f.flyoutId === action.flyoutId ); if (!removedFlyout) { - console.log('[EUI REDUCER] ACTION_CLOSE: Flyout not found, returning state unchanged'); return state; } - console.log('[EUI REDUCER] ACTION_CLOSE: Found flyout to remove:', { - flyoutId: action.flyoutId, - level: removedFlyout.level, - }); - if (removedFlyout.level === LEVEL_MAIN) { // Find the session that contains this main flyout const sessionToRemove = state.sessions.find( @@ -200,12 +157,6 @@ export function flyoutManagerReducer( ); if (sessionToRemove) { - console.log('[EUI REDUCER] ACTION_CLOSE: Removing main flyout session:', { - flyoutId: action.flyoutId, - sessionToRemove, - timestamp: Date.now(), - }); - // Remove all flyouts associated with this session (main + child) const flyoutsToRemove = new Set([action.flyoutId]); if (sessionToRemove.childFlyoutId) { @@ -227,18 +178,6 @@ export function flyoutManagerReducer( newCurrentZIndex = 0; } - console.log('[EUI REDUCER] ACTION_CLOSE: Returning new state after main flyout removal:', { - flyoutId: action.flyoutId, - newSessionsCount: newSessions.length, - newFlyoutsCount: newFlyouts.length, - remainingSessions: newSessions.map((s) => ({ - mainId: s.mainFlyoutId, - childId: s.childFlyoutId, - title: s.title, - })), - timestamp: Date.now(), - }); - return { ...state, sessions: newSessions, @@ -249,13 +188,11 @@ export function flyoutManagerReducer( } // Handle child flyout closing (existing logic) - console.log('[EUI REDUCER] ACTION_CLOSE: Handling child flyout closure'); const newFlyouts = state.flyouts.filter( (f) => f.flyoutId !== action.flyoutId ); if (state.sessions.length === 0) { - console.log('[EUI REDUCER] ACTION_CLOSE: No sessions, returning state with updated flyouts'); return { ...state, flyouts: newFlyouts }; } @@ -271,13 +208,6 @@ export function flyoutManagerReducer( }; } - console.log('[EUI REDUCER] ACTION_CLOSE: Returning new state after child flyout removal:', { - flyoutId: action.flyoutId, - sessionsCount: updatedSessions.length, - flyoutsCount: newFlyouts.length, - timestamp: Date.now(), - }); - return { ...state, sessions: updatedSessions, flyouts: newFlyouts }; } diff --git a/packages/eui/src/components/portal/portal.tsx b/packages/eui/src/components/portal/portal.tsx index 37fcc42e1362..174cd19ea3c1 100644 --- a/packages/eui/src/components/portal/portal.tsx +++ b/packages/eui/src/components/portal/portal.tsx @@ -81,13 +81,6 @@ export class EuiPortalClass extends Component { sibling.insertAdjacentElement(insertPositions[position], portalNode); } - console.log('[EUI PORTAL] componentDidMount:', { - hasPortalNode: !!portalNode, - hasParent: !!portalNode.parentNode, - inDocument: document.body.contains(portalNode), - timestamp: Date.now(), - }); - this.setThemeColor(portalNode); this.updatePortalRef(portalNode); @@ -100,25 +93,9 @@ export class EuiPortalClass extends Component { componentWillUnmount() { const { portalNode } = this.state; - console.log('[EUI PORTAL] componentWillUnmount START:', { - hasPortalNode: !!portalNode, - hasParent: !!(portalNode?.parentNode), - inDocument: portalNode ? document.body.contains(portalNode) : false, - timestamp: Date.now(), - }); if (portalNode?.parentNode) { - console.log('[EUI PORTAL] About to removeChild:', { - parentNode: portalNode.parentNode, - inDocument: document.body.contains(portalNode), - timestamp: Date.now(), - }); portalNode.parentNode.removeChild(portalNode); - console.log('[EUI PORTAL] removeChild completed:', { - timestamp: Date.now(), - }); - } else { - console.log('[EUI PORTAL] Skipping removeChild (no parent node)'); } this.updatePortalRef(null); } From a298fa70f19b1714d775c37f094a52d42900dbea Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 4 Feb 2026 14:39:28 -0700 Subject: [PATCH 5/9] cleanup --- .../eui/src/components/flyout/manager/flyout_managed.tsx | 9 +-------- packages/eui/src/components/portal/portal.tsx | 1 - 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.tsx index d1923ce27535..ca23b4cac09a 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.tsx @@ -179,7 +179,7 @@ export const EuiManagedFlyout = forwardRef( // Track flyoutExistsInManager in a ref to avoid dependency loop // The cleanup function needs the current value but shouldn't cause re-runs const flyoutExistsInManagerRef = useRef(flyoutExistsInManager); - + // Update ref when flyoutExistsInManager changes useEffect(() => { flyoutExistsInManagerRef.current = flyoutExistsInManager; @@ -200,8 +200,6 @@ export const EuiManagedFlyout = forwardRef( // Reset navigation tracking when explicitly closed via isOpen=false wasRegisteredRef.current = false; }; - // CRITICAL: flyoutExistsInManager removed from dependencies - // to prevent re-registration loops during unmount }, [flyoutId, title, level, size, addFlyout, closeFlyout]); // Detect when flyout has been removed from manager state (e.g., via Back button) @@ -236,11 +234,6 @@ export const EuiManagedFlyout = forwardRef( } }, [currentSession, flyoutId, level]); - // REMOVED: Secondary cleanup effect was causing duplicate closeFlyout calls - // which contributed to race conditions during portal cleanup. - // The primary cleanup effect (lines 172-211) is sufficient. - // See: FLYOUT_BUG_ANALYSIS.md for details on the race condition fix. - // Track width changes for flyouts const { width } = useResizeObserver(isActive ? flyoutRef : null, 'width'); diff --git a/packages/eui/src/components/portal/portal.tsx b/packages/eui/src/components/portal/portal.tsx index 174cd19ea3c1..f4f24ba9d214 100644 --- a/packages/eui/src/components/portal/portal.tsx +++ b/packages/eui/src/components/portal/portal.tsx @@ -93,7 +93,6 @@ export class EuiPortalClass extends Component { componentWillUnmount() { const { portalNode } = this.state; - if (portalNode?.parentNode) { portalNode.parentNode.removeChild(portalNode); } From 80253260a2ed229d50efa1553dd00e71483bad5b Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Wed, 4 Feb 2026 15:18:52 -0700 Subject: [PATCH 6/9] add unit test --- .../flyout/manager/flyout_managed.test.tsx | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx index a7c4d36b9314..284c44f1c61d 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx @@ -8,9 +8,20 @@ /* eslint-disable @typescript-eslint/no-var-requires */ +jest.mock('react-dom', () => { + const actual = jest.requireActual('react-dom'); + const mockFlushSync = jest.fn((callback: () => void) => callback()); + return { + ...actual, + flushSync: mockFlushSync, + __mockFlushSync: mockFlushSync, // Export for test access + }; +}); + import React from 'react'; import { act } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; +import * as ReactDOM from 'react-dom'; import { render } from '../../../test/rtl'; import { requiredProps } from '../../../test/required_props'; @@ -23,6 +34,8 @@ import { PROPERTY_LEVEL, } from './const'; +const mockFlushSync = (ReactDOM as any).__mockFlushSync; + // Mock base flyout to a simple div to avoid complex internals jest.mock('../flyout.component', () => { const React = require('react'); @@ -595,5 +608,82 @@ describe('EuiManagedFlyout', () => { expect(mockCloseFlyout).toHaveBeenCalledWith('main-flyout'); expect(onCloseMain).toHaveBeenCalled(); }); + + it('uses flushSync to ensure synchronous state update before DOM cleanup', () => { + const onClose = jest.fn(); + + const { getByTestSubject } = renderInProvider( + + ); + + // Clear any setup calls + mockFlushSync.mockClear(); + mockCloseFlyout.mockClear(); + + // Trigger close via user interaction + act(() => { + userEvent.click(getByTestSubject('managed-flyout')); + }); + + // Verify flushSync was called + expect(mockFlushSync).toHaveBeenCalledTimes(1); + expect(mockFlushSync).toHaveBeenCalledWith(expect.any(Function)); + + // Verify closeFlyout was called (inside flushSync) + expect(mockCloseFlyout).toHaveBeenCalledWith('flush-sync-test'); + + // Verify onClose was called after the synchronous state update + expect(onClose).toHaveBeenCalled(); + }); + + it('calls closeFlyout inside flushSync callback', () => { + const onClose = jest.fn(); + const callOrder: string[] = []; + + // Track execution order + mockFlushSync.mockImplementation((callback: () => void) => { + callOrder.push('flushSync-start'); + callback(); + callOrder.push('flushSync-end'); + }); + + mockCloseFlyout.mockImplementation(() => { + callOrder.push('closeFlyout'); + }); + + onClose.mockImplementation(() => { + callOrder.push('onClose'); + }); + + const { getByTestSubject } = renderInProvider( + + ); + + // Clear setup + callOrder.length = 0; + + // Trigger close + act(() => { + userEvent.click(getByTestSubject('managed-flyout')); + }); + + // Verify closeFlyout is called INSIDE flushSync, and onClose is called AFTER + expect(callOrder).toEqual([ + 'flushSync-start', + 'closeFlyout', + 'flushSync-end', + 'onClose', + ]); + }); }); }); From 23c0cabee0f3e64064949530e53ba0377b13f962 Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 6 Feb 2026 08:30:10 -0700 Subject: [PATCH 7/9] apply suggestion to remove mock export hack --- .../eui/src/components/flyout/manager/flyout_managed.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx index 284c44f1c61d..1a741336f958 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx @@ -14,7 +14,6 @@ jest.mock('react-dom', () => { return { ...actual, flushSync: mockFlushSync, - __mockFlushSync: mockFlushSync, // Export for test access }; }); @@ -34,7 +33,7 @@ import { PROPERTY_LEVEL, } from './const'; -const mockFlushSync = (ReactDOM as any).__mockFlushSync; +const mockFlushSync: jest.Mock = jest.mocked(ReactDOM.flushSync); // Mock base flyout to a simple div to avoid complex internals jest.mock('../flyout.component', () => { From 5ab84f9db57ab4121e36c1ad688b39023f25e70a Mon Sep 17 00:00:00 2001 From: Timothy Sullivan Date: Fri, 6 Feb 2026 08:35:15 -0700 Subject: [PATCH 8/9] eliminate redundant module loading --- .../flyout/manager/flyout_managed.test.tsx | 39 +++++++++---------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx index 1a741336f958..210649196972 100644 --- a/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx +++ b/packages/eui/src/components/flyout/manager/flyout_managed.test.tsx @@ -58,12 +58,15 @@ jest.mock('../flyout.component', () => { // Shared mock functions - must be defined in module scope for Jest const mockCloseFlyout = jest.fn(); -const createMockState = () => ({ + +// Create mock state and functions once at module scope to avoid redundant object creation +const mockState = { sessions: [], flyouts: [], layoutMode: 'side-by-side' as const, -}); -const createMockFunctions = () => ({ +}; + +const mockFunctions = { dispatch: jest.fn(), addFlyout: jest.fn(), closeFlyout: mockCloseFlyout, @@ -72,14 +75,16 @@ const createMockFunctions = () => ({ goBack: jest.fn(), goToFlyout: jest.fn(), historyItems: [], -}); +}; + +const mockFlyoutManager = { + state: mockState, + ...mockFunctions, +}; // Mock hooks that would otherwise depend on ResizeObserver or animation timing jest.mock('./hooks', () => ({ - useFlyoutManager: () => ({ - state: createMockState(), - ...createMockFunctions(), - }), + useFlyoutManager: () => mockFlyoutManager, useIsFlyoutActive: () => true, useHasChildFlyout: () => false, useParentFlyoutSize: () => 'm', @@ -113,10 +118,7 @@ jest.mock('./validation', () => ({ jest.mock('./provider', () => ({ ...jest.requireActual('./provider'), - useFlyoutManager: () => ({ - state: createMockState(), - ...createMockFunctions(), - }), + useFlyoutManager: () => mockFlyoutManager, })); // Mock resize observer hook to return a fixed width @@ -124,6 +126,9 @@ jest.mock('../../observer/resize_observer', () => ({ useResizeObserver: () => ({ width: 480 }), })); +// Cache the actual validation module for tests that need to temporarily restore it +const actualValidation = jest.requireActual('./validation'); + describe('EuiManagedFlyout', () => { const renderInProvider = (ui: React.ReactElement) => render({ui}); @@ -381,13 +386,10 @@ describe('EuiManagedFlyout', () => { describe('size handling', () => { it('defaults main flyout size to "m" when no size is provided', () => { - // Import the real validation function to test the actual behavior - const { validateManagedFlyoutSize } = jest.requireActual('./validation'); - // Temporarily restore the real validation function for this test const originalMock = require('./validation').validateManagedFlyoutSize; require('./validation').validateManagedFlyoutSize = - validateManagedFlyoutSize; + actualValidation.validateManagedFlyoutSize; const { getByTestSubject } = renderInProvider( { }); it('defaults child flyout size to "s" when no size is provided', () => { - // Import the real validation function to test the actual behavior - const { validateManagedFlyoutSize } = jest.requireActual('./validation'); - // Temporarily restore the real validation function for this test const originalMock = require('./validation').validateManagedFlyoutSize; require('./validation').validateManagedFlyoutSize = - validateManagedFlyoutSize; + actualValidation.validateManagedFlyoutSize; const { getByTestSubject } = renderInProvider( Date: Fri, 6 Feb 2026 08:58:43 -0700 Subject: [PATCH 9/9] add changelog --- packages/eui/changelogs/upcoming/9356.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 packages/eui/changelogs/upcoming/9356.md diff --git a/packages/eui/changelogs/upcoming/9356.md b/packages/eui/changelogs/upcoming/9356.md new file mode 100644 index 000000000000..e9fba406df94 --- /dev/null +++ b/packages/eui/changelogs/upcoming/9356.md @@ -0,0 +1,3 @@ +**Bug fixes** + +- Fixed a potential crash in the flyout system: due to asynchronous state updates and React's batching behavior, it was possible to experience a crash when closing a managed flyout.