From 100952eeacdb788bd25362adb7c5e5bdb62d45a7 Mon Sep 17 00:00:00 2001 From: Lenz Weber-Tronic Date: Thu, 4 Apr 2024 12:08:24 +0200 Subject: [PATCH] 0.10.0 (3) switch rehydration mismatch protection to `useSyncExternalStore` (#207) * add test for race condition * switch to `useSyncExternalStore` * add comments * fix up tests * move React functions out of test * disable semgrep * Update packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.test.tsx Co-authored-by: Jerel Miller * remove some `await test` calls --------- Co-authored-by: Jerel Miller --- .../AccumulateMultipartResponsesLink.test.ts | 6 +- .../WrappedApolloClient.test.tsx | 133 +++++++++++++++++- .../useTransportValue.tsx | 16 ++- .../src/util/hydrationTest.ts | 33 +++++ 4 files changed, 173 insertions(+), 15 deletions(-) create mode 100644 packages/client-react-streaming/src/util/hydrationTest.ts diff --git a/packages/client-react-streaming/src/AccumulateMultipartResponsesLink.test.ts b/packages/client-react-streaming/src/AccumulateMultipartResponsesLink.test.ts index 2b5473f9..fdaa8100 100644 --- a/packages/client-react-streaming/src/AccumulateMultipartResponsesLink.test.ts +++ b/packages/client-react-streaming/src/AccumulateMultipartResponsesLink.test.ts @@ -14,7 +14,7 @@ import type { SubscriptionObserver } from "zen-observable-ts"; const { DebounceMultipartResponsesLink: AccumulateMultipartResponsesLink } = await import("#bundled"); -await test("normal queries can resolve synchronously", () => { +test("normal queries can resolve synchronously", () => { const query = gql` query { fastField @@ -46,7 +46,7 @@ await test("normal queries can resolve synchronously", () => { }); }); -await test("deferred query will complete synchonously if maxDelay is 0", () => { +test("deferred query will complete synchonously if maxDelay is 0", () => { const query = gql` query { fastField @@ -81,7 +81,7 @@ await test("deferred query will complete synchonously if maxDelay is 0", () => { }); }); -await test("`next` call will be debounced and results will be merged together", () => { +test("`next` call will be debounced and results will be merged together", () => { mock.timers.enable(); const query = gql` diff --git a/packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.test.tsx b/packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.test.tsx index 12890984..6a6ea6a6 100644 --- a/packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.test.tsx +++ b/packages/client-react-streaming/src/DataTransportAbstraction/WrappedApolloClient.test.tsx @@ -1,4 +1,4 @@ -import React, { Suspense, useMemo } from "rehackt"; +import React, { Suspense, use, useMemo } from "rehackt"; import { outsideOf } from "../util/runInConditions.js"; import assert from "node:assert"; import test, { afterEach, describe } from "node:test"; @@ -24,17 +24,21 @@ const { InMemoryCache, WrapApolloProvider, DataTransportContext, + resetApolloSingletons, } = await import("#bundled"); -await describe( +describe( "tests with DOM access", { skip: outsideOf("node", "browser") }, async () => { // @ts-expect-error seems to have a wrong type? await import("global-jsdom/register"); - const { render, cleanup } = await import("@testing-library/react"); + const { render, cleanup, getQueriesForElement } = await import( + "@testing-library/react" + ); afterEach(cleanup); + afterEach(resetApolloSingletons); const QUERY_ME: TypedDocumentNode<{ me: string }> = gql` query { @@ -215,9 +219,9 @@ await describe( await findByText("User"); assert.ok(attemptedRenderCount > 0); - // one render to rehydrate the server value + // will try with server value and immediately restart with client value // one rerender with the actual client value (which is hopefull equal) - assert.equal(finishedRenderCount, 2); + assert.equal(finishedRenderCount, 1); assert.deepStrictEqual(JSON.parse(JSON.stringify(client.extract())), { ROOT_QUERY: { @@ -227,10 +231,127 @@ await describe( }); } ); + + test( + "race condition: client ahead of server renders without hydration mismatch", + { skip: outsideOf("browser") }, + async () => { + const { $RC, $RS, setBody, hydrateBody, appendToBody } = await import( + "../util/hydrationTest.js" + ); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint + let useStaticValueRefStub = (): { current: T } => { + throw new Error("Should not be called yet!"); + }; + + const client = new ApolloClient({ + connectToDevTools: false, + cache: new InMemoryCache(), + }); + const simulateRequestStart = client.onQueryStarted!; + const simulateRequestData = client.onQueryProgress!; + + const Provider = WrapApolloProvider(({ children }) => { + return ( + ({ + useStaticValueRef() { + return useStaticValueRefStub(); + }, + }), + [] + )} + > + {children} + + ); + }); + + const finishedRenders: any[] = []; + + function Child() { + const { data } = useSuspenseQuery(QUERY_ME); + finishedRenders.push(data); + return
{data.me}
; + } + + const promise = Promise.resolve(); + // suspends on the server, immediately resolved in browser + function ParallelSuspending() { + use(promise); + return
suspending in parallel
; + } + + const { findByText } = getQueriesForElement(document.body); + + // server starts streaming + setBody`Fallback`; + // request started on the server + simulateRequestStart(EVENT_STARTED); + + hydrateBody( + client}> + + + + + + ); + + await findByText("Fallback"); + // this is the div for the suspense boundary + appendToBody``; + // request has finished on the server + simulateRequestData(EVENT_DATA); + simulateRequestData(EVENT_COMPLETE); + // `Child` component wants to transport data from SSR render to the browser + useStaticValueRefStub = () => ({ current: FIRST_HOOK_RESULT as any }); + // `Child` finishes rendering on the server + appendToBody``; + $RS("S:1", "P:1"); + + // meanwhile, in the browser, the cache is modified + client.cache.writeQuery({ + query: QUERY_ME, + data: { + me: "Future me.", + }, + }); + + // `ParallelSuspending` finishes rendering + appendToBody``; + $RS("S:2", "P:2"); + + // everything in the suspense boundary finished rendering, so assemble HTML and take up React rendering again + $RC("B:0", "S:0"); + + // we expect the *new* value to appear after hydration finished, not the old value from the server + await findByText("Future me."); + + // one render to rehydrate the server value + // one rerender with the actual client value (which is hopefull equal) + assert.deepStrictEqual(finishedRenders, [ + { me: "User" }, + { me: "Future me." }, + ]); + + assert.deepStrictEqual(JSON.parse(JSON.stringify(client.extract())), { + ROOT_QUERY: { + __typename: "Query", + me: "Future me.", + }, + }); + assert.equal( + document.body.innerHTML, + `
Future me.
suspending in parallel
` + ); + } + ); } ); -await describe("document transforms are applied correctly", async () => { +describe("document transforms are applied correctly", async () => { const untransformedQuery = gql` query Test { user { diff --git a/packages/client-react-streaming/src/DataTransportAbstraction/useTransportValue.tsx b/packages/client-react-streaming/src/DataTransportAbstraction/useTransportValue.tsx index 5d263e3f..064ee319 100644 --- a/packages/client-react-streaming/src/DataTransportAbstraction/useTransportValue.tsx +++ b/packages/client-react-streaming/src/DataTransportAbstraction/useTransportValue.tsx @@ -1,5 +1,5 @@ "use client"; -import { useContext, useEffect, useState } from "react"; +import { useContext, useSyncExternalStore } from "react"; import { DataTransportContext } from "./DataTransportAbstraction.js"; /** @@ -12,20 +12,24 @@ import { DataTransportContext } from "./DataTransportAbstraction.js"; * the component can change to client-side values instead. */ export function useTransportValue(value: T): T { - const [isClient, setIsClient] = useState(false); - useEffect(() => setIsClient(true), []); - const dataTransport = useContext(DataTransportContext); if (!dataTransport) throw new Error( "useTransportValue must be used within a streaming-specific ApolloProvider" ); const valueRef = dataTransport.useStaticValueRef(value); - if (isClient) { + + const retVal = useSyncExternalStore( + () => () => {}, + () => value, + () => valueRef.current + ); + + if (retVal === value) { // @ts-expect-error this value will never be used again // so we can safely delete it valueRef.current = undefined; } - return isClient ? value : valueRef.current; + return retVal; } diff --git a/packages/client-react-streaming/src/util/hydrationTest.ts b/packages/client-react-streaming/src/util/hydrationTest.ts new file mode 100644 index 00000000..fe4f1829 --- /dev/null +++ b/packages/client-react-streaming/src/util/hydrationTest.ts @@ -0,0 +1,33 @@ +import { hydrateRoot } from "react-dom/client"; + +/* eslint-disable */ +// prettier-ignore +/** React completeSegment function */ +// @ts-expect-error This is React code. +export function $RS(a, b) { a = document.getElementById(a); b = document.getElementById(b); for (a.parentNode.removeChild(a); a.firstChild;)b.parentNode.insertBefore(a.firstChild, b); b.parentNode.removeChild(b) } +// prettier-ignore +/** React completeBoundary function */ +// @ts-expect-error This is React code. +export function $RC(b, c, e = undefined) { c = document.getElementById(c); c.parentNode.removeChild(c); var a = document.getElementById(b); if (a) { b = a.previousSibling; if (e) b.data = "$!", a.setAttribute("data-dgst", e); else { e = b.parentNode; a = b.nextSibling; var f = 0; do { if (a && 8 === a.nodeType) { var d = a.data; if ("/$" === d) if (0 === f) break; else f--; else "$" !== d && "$?" !== d && "$!" !== d || f++ } d = a.nextSibling; e.removeChild(a); a = d } while (a); for (; c.firstChild;)e.insertBefore(c.firstChild, a); b.data = "$" } b._reactRetry && b._reactRetry() } } +/* eslint-enable */ + +export function hydrateBody( + initialChildren: Parameters[1], + options?: Parameters[2] +) { + return hydrateRoot(document.body, initialChildren, options); +} + +export function setBody(html: TemplateStringsArray) { + if (html.length !== 1) + throw new Error("Expected exactly one template string"); + // nosemgrep + document.body.innerHTML = html[0]; +} + +export function appendToBody(html: TemplateStringsArray) { + if (html.length !== 1) + throw new Error("Expected exactly one template string"); + // nosemgrep + document.body.insertAdjacentHTML("beforeend", html[0]); +}