diff --git a/.changeset/shaky-berries-admire.md b/.changeset/shaky-berries-admire.md new file mode 100644 index 0000000000..e345e55513 --- /dev/null +++ b/.changeset/shaky-berries-admire.md @@ -0,0 +1,6 @@ +--- +'@tanstack/solid-start-client': patch +'@tanstack/start-client-core': patch +--- + +compact deferred hydration private state diff --git a/packages/solid-start-client/src/GenericHydrate.tsx b/packages/solid-start-client/src/GenericHydrate.tsx index e23c057f34..4727afd77e 100644 --- a/packages/solid-start-client/src/GenericHydrate.tsx +++ b/packages/solid-start-client/src/GenericHydrate.tsx @@ -34,10 +34,10 @@ type HydrationMarkerDynamicProps = DynamicProps<'div'> & { [key: `data-${string}`]: string | undefined } type PrefetchController = { - abortController: AbortController - hydrationRequested: boolean - hydrationListeners: Set<() => void> - hydrationResolvePending: boolean + abort: AbortController + requested: boolean + listeners: Set<() => void> + resolvePending: boolean started: boolean promise?: Promise } @@ -107,10 +107,10 @@ export function GenericHydrate(props: InternalHydrateProps) { ) const [prefetchError, setPrefetchError] = Solid.createSignal() const controller: PrefetchController = { - abortController: new AbortController(), - hydrationRequested: false, - hydrationListeners: new Set<() => void>(), - hydrationResolvePending: false, + abort: new AbortController(), + requested: false, + listeners: new Set<() => void>(), + resolvePending: false, started: false, } let didPrefetch = false @@ -127,35 +127,37 @@ export function GenericHydrate(props: InternalHydrateProps) { } const onHydrate = (listener: () => void) => { - if (controller.hydrationRequested) { + if (controller.requested) { listener() return () => {} } - controller.hydrationListeners.add(listener) + controller.listeners.add(listener) return () => { - controller.hydrationListeners.delete(listener) + controller.listeners.delete(listener) } } const requestHydration = () => { - if (!controller.hydrationRequested) { - controller.hydrationRequested = true - controller.hydrationListeners.forEach((listener) => listener()) - controller.hydrationListeners.clear() + if (!controller.requested) { + controller.requested = true + controller.listeners.forEach((listener) => listener()) + controller.listeners.clear() } if (!controller.promise) { resolveGate() return } - if (controller.hydrationResolvePending) return - controller.hydrationResolvePending = true + if (controller.resolvePending) { + return + } + controller.resolvePending = true controller.promise.then( () => resolveGate(), (error) => { - if (!controller.abortController.signal.aborted) { + if (!controller.abort.signal.aborted) { setPrefetchError(() => error) } }, @@ -195,12 +197,12 @@ export function GenericHydrate(props: InternalHydrateProps) { .then(() => currentPrefetchStrategy({ element: markerElement ?? null, - signal: controller.abortController.signal, + signal: controller.abort.signal, preload, waitFor: (strategy) => waitForHydrationPrefetchStrategy(strategy, { element: markerElement ?? null, - signal: controller.abortController.signal, + signal: controller.abort.signal, onHydrate, }), }), @@ -209,7 +211,7 @@ export function GenericHydrate(props: InternalHydrateProps) { controller.promise = promise promise.catch((error) => { - if (!controller.abortController.signal.aborted) { + if (!controller.abort.signal.aborted) { setPrefetchError(() => error) } }) @@ -267,8 +269,8 @@ export function GenericHydrate(props: InternalHydrateProps) { } Solid.onCleanup(() => { - controller.abortController.abort() - controller.hydrationListeners.clear() + controller.abort.abort() + controller.listeners.clear() cleanup() releaseGate(gate) }) diff --git a/packages/start-client-core/src/hydration/runtime.ts b/packages/start-client-core/src/hydration/runtime.ts index b1da55195d..307487c4f9 100644 --- a/packages/start-client-core/src/hydration/runtime.ts +++ b/packages/start-client-core/src/hydration/runtime.ts @@ -115,18 +115,19 @@ export function waitForHydrationPrefetchStrategy( } return new Promise((resolve) => { - const state = { disposed: false } - const cleanupStrategyRef: { current: void | (() => void) } = { - current: undefined, - } + let disposed = false + // The strategy may finish synchronously before returning its cleanup. + let cleanupStrategy: void | (() => void) = undefined let cleanupHydrate = () => {} const finish = (reason: HydrationPrefetchWaitReason) => { - if (state.disposed) return - state.disposed = true + if (disposed) { + return + } + disposed = true options.signal.removeEventListener('abort', onAbort) cleanupHydrate() - runHydrationStrategyCleanup(cleanupStrategyRef.current)?.() + runHydrationStrategyCleanup(cleanupStrategy)?.() resolve(reason) } @@ -134,12 +135,13 @@ export function waitForHydrationPrefetchStrategy( options.signal.addEventListener('abort', onAbort, { once: true }) cleanupHydrate = options.onHydrate(() => finish('hydrate')) - const cleanupStrategy = strategy._s?.({ + cleanupStrategy = strategy._s?.({ element: options.element, prefetch: () => finish('prefetch'), }) - cleanupStrategyRef.current = cleanupStrategy - if (state.disposed) { + // A synchronous finish must immediately run the cleanup just returned. + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition + if (disposed) { runHydrationStrategyCleanup(cleanupStrategy)?.() } }) diff --git a/packages/start-client-core/src/hydration/visible.ts b/packages/start-client-core/src/hydration/visible.ts index 3c3391b1de..27855c6d68 100644 --- a/packages/start-client-core/src/hydration/visible.ts +++ b/packages/start-client-core/src/hydration/visible.ts @@ -7,18 +7,23 @@ export type VisibleHydrationOptions = { threshold?: number | Array } -type VisibleObserverEntry = { - key: string - observer: IntersectionObserver - elements: Map void>> -} +type VisibleObserverEntry = [ + observer: IntersectionObserver, + elements: Map void>>, +] const observerRegistry = /* @__PURE__ */ new Map() -function cleanupVisibleObserverEntry(observerEntry: VisibleObserverEntry) { - if (observerEntry.elements.size > 0) return - observerEntry.observer.disconnect() - observerRegistry.delete(observerEntry.key) +function cleanupVisibleObserverEntry( + key: string, + observer: IntersectionObserver, + elements: Map void>>, +) { + if (elements.size > 0) { + return + } + observer.disconnect() + observerRegistry.delete(key) } /* @__NO_SIDE_EFFECTS__ */ @@ -44,46 +49,48 @@ export function visible( let observerEntry = observerRegistry.get(key) if (!observerEntry) { - const entry: VisibleObserverEntry = { - key, - elements: new Map void>>(), - observer: new IntersectionObserver( - (entries) => { - for (const intersectingEntry of entries) { - if (!intersectingEntry.isIntersecting) continue - - const callbacks = entry.elements.get(intersectingEntry.target) - if (!callbacks) continue + const elements = new Map void>>() + const observer = new IntersectionObserver( + (entries) => { + for (const intersectingEntry of entries) { + if (!intersectingEntry.isIntersecting) { + continue + } - callbacks.forEach((callback) => callback()) - entry.elements.delete(intersectingEntry.target) - entry.observer.unobserve(intersectingEntry.target) - cleanupVisibleObserverEntry(entry) + const callbacks = elements.get(intersectingEntry.target) + if (!callbacks) { + continue } - }, - { rootMargin, threshold }, - ), - } - observerRegistry.set(key, entry) - observerEntry = entry + + callbacks.forEach((callback) => callback()) + elements.delete(intersectingEntry.target) + observer.unobserve(intersectingEntry.target) + cleanupVisibleObserverEntry(key, observer, elements) + } + }, + { rootMargin, threshold }, + ) + observerEntry = [observer, elements] + observerRegistry.set(key, observerEntry) } - let callbacks = observerEntry.elements.get(element) + const [observer, elements] = observerEntry + let callbacks = elements.get(element) if (!callbacks) { callbacks = new Set() - observerEntry.elements.set(element, callbacks) - observerEntry.observer.observe(element) + elements.set(element, callbacks) + observer.observe(element) } callbacks.add(callback) return () => { - const currentCallbacks = observerEntry.elements.get(element) + const currentCallbacks = elements.get(element) currentCallbacks?.delete(callback) if (currentCallbacks?.size === 0) { - observerEntry.elements.delete(element) - observerEntry.observer.unobserve(element) + elements.delete(element) + observer.unobserve(element) } - cleanupVisibleObserverEntry(observerEntry) + cleanupVisibleObserverEntry(key, observer, elements) } }, } diff --git a/packages/start-client-core/tests/hydration-runtime.test.ts b/packages/start-client-core/tests/hydration-runtime.test.ts new file mode 100644 index 0000000000..577d329cd9 --- /dev/null +++ b/packages/start-client-core/tests/hydration-runtime.test.ts @@ -0,0 +1,100 @@ +import { describe, expect, it, vi } from 'vitest' +import { waitForHydrationPrefetchStrategy } from '../src/hydration/runtime' +import type { HydrationPrefetchStrategy } from '../src/hydration/types' + +describe('waitForHydrationPrefetchStrategy', () => { + it('cleans up a strategy that finishes during synchronous setup', async () => { + const abortController = new AbortController() + const cleanupHydrate = vi.fn() + const cleanupStrategy = vi.fn() + let hydrate = () => {} + + const strategy: HydrationPrefetchStrategy = { + _s: ({ prefetch }) => { + prefetch?.() + return cleanupStrategy + }, + } + + const result = waitForHydrationPrefetchStrategy(strategy, { + element: null, + signal: abortController.signal, + onHydrate: (listener) => { + hydrate = listener + return cleanupHydrate + }, + }) + + await expect(result).resolves.toBe('prefetch') + expect(cleanupHydrate).toHaveBeenCalledTimes(1) + expect(cleanupStrategy).toHaveBeenCalledTimes(1) + + hydrate() + abortController.abort() + expect(cleanupHydrate).toHaveBeenCalledTimes(1) + expect(cleanupStrategy).toHaveBeenCalledTimes(1) + }) + + it.each([ + ['abort', 'abort'], + ['hydrate', 'hydrate'], + ] as const)( + 'settles an abort/hydrate race with %s first and cleans up once', + async (winner, expectedReason) => { + const abortController = new AbortController() + const cleanupHydrate = vi.fn() + const cleanupStrategy = vi.fn() + let hydrate = () => {} + let prefetch = () => {} + + const strategy: HydrationPrefetchStrategy = { + _s: (context) => { + prefetch = context.prefetch ?? (() => {}) + return cleanupStrategy + }, + } + + const result = waitForHydrationPrefetchStrategy(strategy, { + element: null, + signal: abortController.signal, + onHydrate: (listener) => { + hydrate = listener + return cleanupHydrate + }, + }) + + if (winner === 'abort') { + abortController.abort() + hydrate() + } else { + hydrate() + abortController.abort() + } + prefetch() + + await expect(result).resolves.toBe(expectedReason) + expect(cleanupHydrate).toHaveBeenCalledTimes(1) + expect(cleanupStrategy).toHaveBeenCalledTimes(1) + }, + ) + + it('does not set up a strategy when the signal is already aborted', async () => { + const abortController = new AbortController() + abortController.abort() + const setup = vi.fn() + const onHydrate = vi.fn() + + const result = waitForHydrationPrefetchStrategy( + { _s: setup }, + { + element: null, + signal: abortController.signal, + onHydrate, + }, + ) + + await expect(result).resolves.toBe('abort') + expect(setup).not.toHaveBeenCalled() + expect(onHydrate).not.toHaveBeenCalled() + }) +}) diff --git a/packages/start-client-core/tests/hydration-visible.test.ts b/packages/start-client-core/tests/hydration-visible.test.ts new file mode 100644 index 0000000000..c0fbac0dfd --- /dev/null +++ b/packages/start-client-core/tests/hydration-visible.test.ts @@ -0,0 +1,127 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { visible } from '../src/hydration/visible' +import type { HydrationPrefetchStrategy } from '../src/hydration/types' + +class IntersectionObserverMock implements IntersectionObserver { + readonly root: Document | Element | null + readonly rootMargin: string + readonly scrollMargin: string + readonly thresholds: ReadonlyArray + readonly observe = vi.fn((_target: Element) => {}) + readonly unobserve = vi.fn((_target: Element) => {}) + readonly disconnect = vi.fn(() => {}) + + constructor( + readonly callback: IntersectionObserverCallback, + options: IntersectionObserverInit = {}, + ) { + this.root = options.root ?? null + this.rootMargin = options.rootMargin ?? '0px' + this.scrollMargin = options.scrollMargin ?? '0px' + this.thresholds = Array.isArray(options.threshold) + ? options.threshold + : [options.threshold ?? 0] + } + + takeRecords(): Array { + return [] + } + + emit(target: Element, isIntersecting = true) { + this.callback( + [{ target, isIntersecting } as IntersectionObserverEntry], + this, + ) + } +} + +describe('visible hydration strategy', () => { + let observers: Array + const cleanups: Array<() => void> = [] + + beforeEach(() => { + observers = [] + vi.stubGlobal( + 'IntersectionObserver', + class extends IntersectionObserverMock { + constructor( + callback: IntersectionObserverCallback, + options?: IntersectionObserverInit, + ) { + super(callback, options) + observers.push(this) + } + }, + ) + }) + + afterEach(() => { + cleanups.splice(0).forEach((cleanup) => cleanup()) + vi.unstubAllGlobals() + }) + + function observe( + strategy: HydrationPrefetchStrategy, + element: Element, + callback: () => void, + ) { + const cleanup = strategy._s?.({ element, prefetch: callback }) + if (cleanup) { + cleanups.push(cleanup) + } + return cleanup + } + + it('shares an observer and tracks multiple callbacks for one element', () => { + const element = document.createElement('div') + const first = vi.fn() + const second = vi.fn() + const strategy = visible({ rootMargin: '25px', threshold: [0, 0.5] }) + + const cleanupFirst = observe(strategy, element, first) + observe(strategy, element, second) + + expect(observers).toHaveLength(1) + expect(observers[0]!.observe).toHaveBeenCalledOnce() + expect(observers[0]!.observe).toHaveBeenCalledWith(element) + + cleanupFirst?.() + observers[0]!.emit(element) + + expect(first).not.toHaveBeenCalled() + expect(second).toHaveBeenCalledOnce() + expect(observers[0]!.unobserve).toHaveBeenCalledOnce() + expect(observers[0]!.unobserve).toHaveBeenCalledWith(element) + expect(observers[0]!.disconnect).toHaveBeenCalledOnce() + }) + + it('keeps a shared observer until every element is cleaned up', () => { + const firstElement = document.createElement('div') + const secondElement = document.createElement('div') + const options = { rootMargin: '50px', threshold: 0.25 } + + const cleanupFirst = observe(visible(options), firstElement, vi.fn()) + const cleanupSecond = observe(visible(options), secondElement, vi.fn()) + + expect(observers).toHaveLength(1) + expect(observers[0]!.observe).toHaveBeenCalledTimes(2) + + cleanupFirst?.() + expect(observers[0]!.unobserve).toHaveBeenCalledWith(firstElement) + expect(observers[0]!.disconnect).not.toHaveBeenCalled() + + cleanupSecond?.() + expect(observers[0]!.unobserve).toHaveBeenCalledWith(secondElement) + expect(observers[0]!.disconnect).toHaveBeenCalledOnce() + + const cleanupThird = observe( + visible(options), + document.createElement('div'), + vi.fn(), + ) + expect(observers).toHaveLength(2) + + cleanupThird?.() + expect(observers[1]!.disconnect).toHaveBeenCalledOnce() + }) +})