diff --git a/src/workspaces/officerdev/src/apps/Preview/PreviewApp.test.tsx b/src/workspaces/officerdev/src/apps/Preview/PreviewApp.test.tsx new file mode 100644 index 00000000..15b1c5b4 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Preview/PreviewApp.test.tsx @@ -0,0 +1,110 @@ +import type { ReactNode } from 'react'; +import { afterEach, describe, expect, test } from 'bun:test'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { render, waitFor } from '@testing-library/react'; +import { DevProxyFrame } from './PreviewApp'; + +/** + * Whether the frame is REMOUNTED, which is the same question as whether the app reloads. + * + * Both bugs this pins were invisible to reading the component and obvious in the DOM. The first shipped: + * changing route inside a preview reloaded the whole app about a second later — the debounce on writing the + * location down, feeding a URL into an effect whose `setReady(false)` tore the frame out of the tree. The + * second was introduced by the fix for the first: with `src` frozen at mount, nothing remounted when a + * person typed a NEW url, so the panel simply ignored it. + * + * They pull in opposite directions, so a test for either alone would have been passed by the other bug. + * + * `fetch` is stubbed rather than `mock.module('hooks/useClient')`: Bun has one module registry for the whole + * run, so that mock is process-wide and permanent, and `useDashboardState.test.tsx` already owns it. A global + * restored in `afterEach` cannot reach across files the same way. + */ +const realFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = realFetch; +}); + +const stubFetch = () => { + globalThis.fetch = (async () => + new Response(JSON.stringify({ ok: true }), { + status: 200, + headers: { 'content-type': 'application/json' }, + })) as unknown as typeof fetch; +}; + +const PROXY = '/api/dev-proxy/3001/'; +const frame = (el: HTMLElement) => el.querySelector('iframe'); + +// `useClient` reaches for auth through react-query, so the component needs a provider even though nothing +// here is querying. +const Wrap = ({ children }: { children: ReactNode }) => ( + + {children} + +); + +const mount = (props: { entry: string; target: string; reloadToken?: number }) => + render( + + {}} + /> + , + ); + +describe('DevProxyFrame remounting', () => { + test('the app navigating does NOT reload the frame', async () => { + stubFetch(); + const view = mount({ entry: PROXY, target: PROXY }); + await waitFor(() => expect(frame(view.container)).not.toBeNull()); + const before = frame(view.container); + + // What the location reporter produces a debounce after a route change. + view.rerender( + + {}} /> + , + ); + + // Same DOM node = the browser never re-requested anything. A remount here is the reported bug. + expect(frame(view.container)).toBe(before); + expect(frame(view.container)?.getAttribute('src')).toBe(PROXY); + }); + + test('a newly typed url DOES load — the regression the freeze introduced', async () => { + stubFetch(); + const view = mount({ entry: PROXY, target: PROXY }); + await waitFor(() => expect(frame(view.container)).not.toBeNull()); + const before = frame(view.container); + + const next = '/api/dev-proxy/4000/'; + view.rerender( + + {}} /> + , + ); + + await waitFor(() => expect(frame(view.container)?.getAttribute('src')).toBe(next)); + expect(frame(view.container)).not.toBe(before); + }); + + test('the reload button remounts, and comes back at the current page', async () => { + stubFetch(); + const deep = '/api/dev-proxy/3001/settings/profile'; + const view = mount({ entry: PROXY, target: deep }); + await waitFor(() => expect(frame(view.container)).not.toBeNull()); + const before = frame(view.container); + + view.rerender( + + {}} /> + , + ); + + await waitFor(() => expect(frame(view.container)).not.toBe(before)); + expect(frame(view.container)?.getAttribute('src')).toBe(deep); + }); +}); diff --git a/src/workspaces/officerdev/src/apps/Preview/PreviewApp.tsx b/src/workspaces/officerdev/src/apps/Preview/PreviewApp.tsx index 390643bf..81167101 100644 --- a/src/workspaces/officerdev/src/apps/Preview/PreviewApp.tsx +++ b/src/workspaces/officerdev/src/apps/Preview/PreviewApp.tsx @@ -56,7 +56,7 @@ export const PreviewApp = ({ panelId }: { panelId: string }) => { ); } - return ; + return ; }; /** @@ -71,14 +71,39 @@ export const PreviewApp = ({ panelId }: { panelId: string }) => { * * An ordinary URL mounts immediately — nothing to arrange, and no reason to make it wait. */ -type DevProxyFrameProps = { url: string; reloadToken: number; onLocation: (path: string) => void }; +/** + * `entry` is what the panel was pointed at; `target` is that plus wherever the app has navigated to. + * + * Both are needed here and they are not interchangeable. `entry` decides WHEN to start over — a person + * typing a new address — and `target` decides WHAT to load. Keying on `target` would restart the frame + * every time the app moved, which is the whole bug below. + */ +type DevProxyFrameProps = { + entry: string; + target: string; + reloadToken: number; + onLocation: (path: string) => void; +}; -const DevProxyFrame = ({ url, reloadToken, onLocation }: DevProxyFrameProps) => { +export const DevProxyFrame = ({ entry, target, reloadToken, onLocation }: DevProxyFrameProps) => { const { get } = useClient(); - const needsSession = url.startsWith('/api/dev-proxy/'); + const needsSession = entry.startsWith('/api/dev-proxy/'); const [ready, setReady] = useState(!needsSession); const [failed, setFailed] = useState(null); + /** + * The session cookie, and why this effect must NOT watch the URL. + * + * It used to depend on the full composed URL, which changes every time the app navigates. Each change + * re-ran this, and `setReady(false)` tears the frame down to show "Connecting…" — so a route change was + * followed, one debounce later, by the whole app reloading. Reported as "I change routes and a second + * later the page refreshes"; the second was the 800ms debounce on writing the location down. + * + * Watching the URL was never right regardless: the cookie is set with `Path=/api/dev-proxy` and no port + * in it, so it already covers every path and every port the panel might point at. The only things that + * warrant a fresh one are entering proxy mode at all, and an explicit reload — where re-requesting is the + * point, since a preview left open overnight should recover by itself rather than show a wall of 401s. + */ useEffect(() => { if (!needsSession) { setReady(true); @@ -87,15 +112,13 @@ const DevProxyFrame = ({ url, reloadToken, onLocation }: DevProxyFrameProps) => let alive = true; setReady(false); setFailed(null); - // Re-run on every reload: the cookie has a lifetime, and a preview left open overnight should recover - // by itself rather than showing a wall of 401s. get<{ ok: boolean }>('/dev-proxy/session') .then(() => alive && setReady(true)) .catch((err: unknown) => alive && setFailed(err instanceof Error ? err.message : 'could not authorise')); return () => { alive = false; }; - }, [needsSession, reloadToken, url]); + }, [needsSession, reloadToken]); if (failed) { return ( @@ -107,7 +130,11 @@ const DevProxyFrame = ({ url, reloadToken, onLocation }: DevProxyFrameProps) => if (!ready) { return
Connecting…
; } - return ; + // Keyed on the ENTRY url as well as the reload counter. Freezing `src` at mount stopped the app's own + // navigation from reloading the frame — and stopped a newly typed URL from loading at all, since nothing + // remounted. The entry url in the key is what distinguishes the two: it changes only when a person points + // the panel somewhere else, and `setUrl` clears the remembered path when they do. + return ; }; /**