From 41e9c17a69f819f20a29823a0acf18456c5ef4d2 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Tue, 29 Aug 2023 10:40:20 +0100 Subject: [PATCH] fix[devtools]: allow element updates polling only if bridge is alive (#27067) When some element is inspected in DevTools, we have a polling which updates the data for this element. Sometimes when service worker dies or bridge is getting shutdown, we continue to poll this data and will spam with the same "timed out error". Screenshot 2023-07-28 at 17 39 23 These changes add an explicit check that polling is allowed only while bridge is alive. --- .../Components/InspectedElementContext.js | 16 +++++- .../src/inspectedElementCache.js | 51 ++++++++++--------- 2 files changed, 42 insertions(+), 25 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js index 13bc39ba2f467..738f92a516af6 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementContext.js @@ -107,6 +107,8 @@ export function InspectedElementContextController({ parseHookNamesByDefault || alreadyLoadedHookNames, ); + const [bridgeIsAlive, setBridgeIsAliveStatus] = useState(true); + const elementHasChanged = element !== null && element !== state.element; // Reset the cached inspected paths when a new element is selected. @@ -213,14 +215,25 @@ export function InspectedElementContextController({ } }, [state]); + useEffect(() => { + // Assuming that new bridge is always alive at this moment + setBridgeIsAliveStatus(true); + + const listener = () => setBridgeIsAliveStatus(false); + bridge.addListener('shutdown', listener); + + return () => bridge.removeListener('shutdown', listener); + }, [bridge]); + // Periodically poll the selected element for updates. useEffect(() => { - if (element !== null) { + if (element !== null && bridgeIsAlive) { const checkForUpdateWrapper = () => { checkForUpdate({bridge, element, refresh, store}); timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); }; let timeoutID = setTimeout(checkForUpdateWrapper, POLL_INTERVAL); + return () => { clearTimeout(timeoutID); }; @@ -232,6 +245,7 @@ export function InspectedElementContextController({ // No sense to ping right away after e.g. inspecting/hydrating a path. inspectedElement, state, + bridgeIsAlive, ]); const value = useMemo( diff --git a/packages/react-devtools-shared/src/inspectedElementCache.js b/packages/react-devtools-shared/src/inspectedElementCache.js index f0fb6c542ef32..6fb8b9d09fc02 100644 --- a/packages/react-devtools-shared/src/inspectedElementCache.js +++ b/packages/react-devtools-shared/src/inspectedElementCache.js @@ -180,32 +180,35 @@ export function checkForUpdate({ }): void { const {id} = element; const rendererID = store.getRendererIDForElement(id); - if (rendererID != null) { - inspectElementMutableSource({ - bridge, - element, - path: null, - rendererID: ((rendererID: any): number), - }).then( - ([inspectedElement, responseType]: [ - InspectedElementFrontend, - InspectedElementResponseType, - ]) => { - if (responseType === 'full-data') { - startTransition(() => { - const [key, value] = createCacheSeed(element, inspectedElement); - refresh(key, value); - }); - } - }, - // There isn't much to do about errors in this case, - // but we should at least log them so they aren't silent. - error => { - console.error(error); - }, - ); + if (rendererID == null) { + return; } + + inspectElementMutableSource({ + bridge, + element, + path: null, + rendererID, + }).then( + ([inspectedElement, responseType]: [ + InspectedElementFrontend, + InspectedElementResponseType, + ]) => { + if (responseType === 'full-data') { + startTransition(() => { + const [key, value] = createCacheSeed(element, inspectedElement); + refresh(key, value); + }); + } + }, + + // There isn't much to do about errors in this case, + // but we should at least log them so they aren't silent. + error => { + console.error(error); + }, + ); } export function clearCacheBecauseOfError(refresh: RefreshFunction): void {