diff --git a/nuxt-app/composables/useLoadingScreen.ts b/nuxt-app/composables/useLoadingScreen.ts index e0de9c4f..00b59ed0 100644 --- a/nuxt-app/composables/useLoadingScreen.ts +++ b/nuxt-app/composables/useLoadingScreen.ts @@ -1,8 +1,5 @@ import type { Ref } from 'vue' -import { reactive, ref, watch } from 'vue' - -// Global is loading state -const isLoading = ref(false) +import { reactive, watch } from 'vue' /** * Composable to set and get the global state of the loading screen. @@ -12,7 +9,15 @@ const isLoading = ref(false) * @returns The state of the loading screen. */ export function useLoadingScreen(...dataList: Ref[]) { - // Set initial state + // `useState`, not a module-scope `ref`: a module-scope ref is created once per server worker and + // shared by every concurrent request, so one visitor's navigation can change what another + // visitor's page renders. `useState` is per-request on the server. + const isLoading = useState('loading-screen', () => false) + + // Called with no arguments this writes `false` rather than reading, which is how + // `LoadingScreen.vue` clears the overlay. Not all pages use/call this composable, so + // without the reset, navigating to one of them from a page that was still loading would leave a + // full-screen overlay stuck. Splitting read from write needs a route-change reset first. isLoading.value = dataList.some((data) => !data.value) // Change state on update diff --git a/nuxt-app/test/useLoadingScreen.test.ts b/nuxt-app/test/useLoadingScreen.test.ts new file mode 100644 index 00000000..db9f11aa --- /dev/null +++ b/nuxt-app/test/useLoadingScreen.test.ts @@ -0,0 +1,71 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { ref, type Ref } from 'vue' + +// `useLoadingScreen` reads Nuxt's `useState`, which is an auto-import at runtime. Stand in a minimal +// version with the two properties that matter: it returns a Vue `ref` (so `reactive()` unwraps it +// exactly as in production), and there is one store per "request", so a value created during a +// request is reused within it and gone in the next. +let store = new Map>() +const newRequest = () => { + store = new Map() +} + +beforeEach(() => { + newRequest() + ;(globalThis as Record).useState = (key: string, init: () => T) => { + if (!store.has(key)) { + store.set(key, ref(init()) as Ref) + } + return store.get(key) as Ref + } +}) + +afterEach(() => { + delete (globalThis as Record).useState +}) + +describe('useLoadingScreen', () => { + it('does not carry state from one request into the next', async () => { + // `isLoading` used to be a module-scope `ref`, created once per server worker and shared by + // every concurrent request, so one visitor's pending navigation could show another visitor a + // loading screen — and make the server's HTML disagree with that visitor's first client render. + const { useLoadingScreen } = await import('../composables/useLoadingScreen') + + const pending = ref(null) + expect(useLoadingScreen(pending).isLoading).toBe(true) + + newRequest() + const loaded = ref({ data: 'ready' }) + expect(useLoadingScreen(loaded).isLoading).toBe(false) + }) + + it('shares one flag between all callers within a request', async () => { + // The page sets the flag by passing its data; `LoadingScreen.vue` calls with no arguments. + // Both must observe the same state or the loading screen cannot work at all. + // + // Note the quirk this pins down: a no-argument call *writes* `false`, because + // `[].some(...)` is false. It reads like a getter and is not one. The order in `app.vue` + // (LoadingScreen before ``) is what stops that clobbering the page's value. + const { useLoadingScreen } = await import('../composables/useLoadingScreen') + + const pending = ref(null) + const fromPage = useLoadingScreen(pending) + expect(fromPage.isLoading).toBe(true) + + useLoadingScreen() + + expect(fromPage.isLoading).toBe(false) + }) + + it('tracks its data list within a request', async () => { + const { useLoadingScreen } = await import('../composables/useLoadingScreen') + + const data = ref(null) + const state = useLoadingScreen(data) + expect(state.isLoading).toBe(true) + + data.value = { data: 'ready' } + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(state.isLoading).toBe(false) + }) +})