From c8595918276b253ac51b0090780f3efc6502c6a3 Mon Sep 17 00:00:00 2001 From: Jamey Huffnagle Date: Wed, 3 Apr 2024 10:38:24 -0400 Subject: [PATCH] fix(app): fix excessive /runs network requests (#14783) Closes EXEC-255 UseNotifyService utilizes hostname instead of the host object, preventing a pass by reference issue causing excessive requests sent to /runs that often occurs on the RobotDetails page. Let's also ensure that if a robot loses connection while a component has passed a callback to appShellListener, we ensure we remove the correct callback from the store. --- .../resources/__tests__/useNotifyService.test.ts | 16 ++++++++++++++++ app/src/resources/useNotifyService.ts | 10 ++++++++++ 2 files changed, 26 insertions(+) diff --git a/app/src/resources/__tests__/useNotifyService.test.ts b/app/src/resources/__tests__/useNotifyService.test.ts index c286bec0b86..c27bebcf8b5 100644 --- a/app/src/resources/__tests__/useNotifyService.test.ts +++ b/app/src/resources/__tests__/useNotifyService.test.ts @@ -228,4 +228,20 @@ describe('useNotifyService', () => { expect.objectContaining({ hostname: MOCK_HOST_CONFIG.hostname }) ) }) + + it('should still clean up the listener if the hostname changes to null after subscribing', () => { + const { unmount, rerender } = renderHook(() => + useNotifyService({ + hostOverride: MOCK_HOST_CONFIG, + topic: MOCK_TOPIC, + setRefetchUsingHTTP: mockHTTPRefetch, + options: MOCK_OPTIONS, + }) + ) + rerender({ hostOverride: null }) + unmount() + expect(appShellListener).toHaveBeenCalledWith( + expect.objectContaining({ hostname: MOCK_HOST_CONFIG.hostname }) + ) + }) }) diff --git a/app/src/resources/useNotifyService.ts b/app/src/resources/useNotifyService.ts index b6a1667b3b9..697d1168477 100644 --- a/app/src/resources/useNotifyService.ts +++ b/app/src/resources/useNotifyService.ts @@ -40,6 +40,11 @@ export function useNotifyService({ const host = hostOverride ?? hostFromProvider const hostname = host?.hostname ?? null const doTrackEvent = useTrackEvent() +<<<<<<< HEAD +======= + const isFlex = useIsFlex(host?.robotName ?? '') + const hasUsedNotifyService = React.useRef(false) +>>>>>>> 61be566d31 (fix(app): fix excessive /runs network requests (#14783)) const seenHostname = React.useRef(null) const { enabled, staleTime, forceHttpPolling } = options @@ -69,6 +74,7 @@ export function useNotifyService({ if (seenHostname.current != null) { ======= hasUsedNotifyService.current = true + seenHostname.current = hostname } else { setRefetchUsingHTTP('always') } @@ -77,7 +83,11 @@ export function useNotifyService({ if (hasUsedNotifyService.current) { >>>>>>> 1ba616651c (refactor(app-shell-odd): Utilize robot-server unsubscribe flags (#14724)) appShellListener({ +<<<<<<< HEAD hostname: seenHostname.current, +======= + hostname: seenHostname.current as string, +>>>>>>> 61be566d31 (fix(app): fix excessive /runs network requests (#14783)) topic, callback: onDataEvent, isDismounting: true,