Skip to content

Commit

Permalink
fix[devtools]: allow element updates polling only if bridge is alive (f…
Browse files Browse the repository at this point in the history
…acebook#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".

<img width="1728" alt="Screenshot 2023-07-28 at 17 39 23"
src="https://github.com/facebook/react/assets/28902667/220c4504-1ccc-4e87-9d78-bfff8b708230">


These changes add an explicit check that polling is allowed only while
bridge is alive.
  • Loading branch information
hoxyq authored and AndyPengc12 committed Apr 15, 2024
1 parent 8aebf07 commit 4edb785
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ export function InspectedElementContextController({
parseHookNamesByDefault || alreadyLoadedHookNames,
);

const [bridgeIsAlive, setBridgeIsAliveStatus] = useState<boolean>(true);

const elementHasChanged = element !== null && element !== state.element;

// Reset the cached inspected paths when a new element is selected.
Expand Down Expand Up @@ -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);
};
Expand All @@ -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<Context>(
Expand Down
51 changes: 27 additions & 24 deletions packages/react-devtools-shared/src/inspectedElementCache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 4edb785

Please sign in to comment.