0.10.0 (1) - Throw error when passing arguments into getClient. (#203)

* Throw error when passing arguments into `getClient`.

* Update packages/client-react-streaming/src/registerApolloClient.tsx

Co-authored-by: Jerel Miller <[email protected]>

* Update packages/client-react-streaming/src/registerApolloClient.tsx

* add unit tests for `registerApolloClient`

* add test for call with parameters

---------

Co-authored-by: Jerel Miller <[email protected]>

Author: phryneas

packages/client-react-streaming/package.json

@@ -134,6 +134,7 @@
     "publint": "0.2.7",
     "react": "18.3.0-canary-60a927d04-20240113",
     "react-error-boundary": "4.0.13",
+    "react-server-dom-webpack": "18.3.0-canary-60a927d04-20240113",
     "rimraf": "5.0.5",
     "superjson": "1.13.3",
     "ts-node": "10.9.2",

packages/client-react-streaming/src/registerApolloClient.test.tsx

@@ -0,0 +1,131 @@
+import { it } from "node:test";
+import assert from "node:assert";
+import { runInConditions } from "./util/runInConditions.js";
+import { Writable } from "node:stream";
+
+runInConditions("react-server");
+
+const { registerApolloClient, ApolloClient, InMemoryCache } = await import(
+  "#bundled"
+);
+
+type ReactServer = {
+  renderToPipeableStream(
+    model: React.ReactNode,
+    webpackMap: unknown,
+    options?: {
+      environmentName?: string;
+      onError?: (error: any) => void;
+      onPostpone?: (reason: string) => void;
+      identifierPrefix?: string;
+    }
+  ): {
+    abort(reason: any): void;
+    pipe<T extends Writable>(destination: T): T;
+  };
+};
+
+const { renderToPipeableStream } = (await import(
+  // @ts-expect-error close enough
+  "react-server-dom-webpack/server"
+)) as ReactServer;
+const React = await import("react");
+
+function drain(stream: ReturnType<typeof renderToPipeableStream>) {
+  let result = "";
+  return new Promise<string>((resolve) => {
+    stream.pipe(
+      new Writable({
+        write(chunk, _encoding, callback) {
+          result += chunk.toString();
+          callback();
+        },
+        final(callback) {
+          resolve(result);
+          callback();
+        },
+      })
+    );
+  });
+}
+
+function makeClient() {
+  return new ApolloClient({
+    cache: new InMemoryCache(),
+    connectToDevTools: false,
+  });
+}
+
+it("calling `getClient` outside of a React render creates a new instance every time", () => {
+  const { getClient } = registerApolloClient(makeClient);
+
+  const client1 = getClient();
+  const client2 = getClient();
+  assert.notStrictEqual(client1, client2);
+});
+
+it("calling `getClient` twice during the same React render will return the same instance", async () => {
+  const { getClient } = registerApolloClient(makeClient);
+
+  const clients: any[] = [];
+  function App() {
+    clients.push(getClient());
+    clients.push(getClient());
+    return <div></div>;
+  }
+
+  const stream = renderToPipeableStream(React.createElement(App), {});
+  await drain(stream);
+
+  assert.equal(clients.length, 2);
+  assert.ok(clients[0] instanceof ApolloClient);
+  assert.strictEqual(clients[0], clients[1]);
+});
+
+it("calling `getClient` twice during different React renders will return different instances", async () => {
+  const { getClient } = registerApolloClient(makeClient);
+
+  const clients: any[] = [];
+  function App() {
+    clients.push(getClient());
+    return <div></div>;
+  }
+
+  {
+    const stream = renderToPipeableStream(React.createElement(App), {});
+    await drain(stream);
+  }
+  {
+    const stream = renderToPipeableStream(React.createElement(App), {});
+    await drain(stream);
+  }
+
+  assert.equal(clients.length, 2);
+  assert.ok(clients[0] instanceof ApolloClient);
+  assert.notStrictEqual(clients[0], clients[1]);
+});
+
+it("calling `getClient` with parameters results in an error", async () => {
+  const { getClient } = registerApolloClient(makeClient);
+
+  function App() {
+    // @ts-expect-error yeah this is a bad idea, that's why we do it in a test
+    getClient("argument");
+    return <div></div>;
+  }
+
+  let error: undefined | Error;
+  const stream = renderToPipeableStream(
+    React.createElement(App),
+    {},
+    {
+      onError(e) {
+        error = e;
+      },
+    }
+  );
+  await drain(stream);
+  assert.ok(
+    /You cannot pass arguments into `getClient`./.test(error?.message || "")
+  );
+});

packages/client-react-streaming/src/registerApolloClient.tsx

@@ -50,7 +50,20 @@ export function registerApolloClient(makeClient: () => ApolloClient<any>): {
 export function registerApolloClient(
   makeClient: (() => Promise<ApolloClient<any>>) | (() => ApolloClient<any>)
 ) {
-  const getClient = cache(makeClient);
+  function wrappedMakeClient() {
+    if (arguments.length) {
+      throw new Error(
+        `
+You cannot pass arguments into \`getClient\`.
+Passing arguments to \`getClient\` returns a different instance
+of Apollo Client each time it is called with different arguments, potentially 
+resulting in duplicate requests and a non-functional cache. 
+      `.trim()
+      );
+    }
+    return makeClient();
+  }
+  const getClient = cache(wrappedMakeClient);
   return {
     getClient,
   };

yarn.lock

@@ -71,6 +71,7 @@ __metadata:
     publint: "npm:0.2.7"
     react: "npm:18.3.0-canary-60a927d04-20240113"
     react-error-boundary: "npm:4.0.13"
+    react-server-dom-webpack: "npm:18.3.0-canary-60a927d04-20240113"
     rimraf: "npm:5.0.5"
     superjson: "npm:1.13.3"
     ts-invariant: "npm:^0.10.3"
@@ -5763,14 +5764,23 @@ __metadata:
   languageName: node
   linkType: hard
 
+"acorn-loose@npm:^8.3.0":
+  version: 8.4.0
+  resolution: "acorn-loose@npm:8.4.0"
+  dependencies:
+    acorn: "npm:^8.11.0"
+  checksum: 10/a005b2bee62e2575963b311ab7c45701062115a62e4286162498b1b198a6f884ceea186592ce41a27d5f382a5b640f1dffb37dd0e6e7848a74dd36e4b0a55105
+  languageName: node
+  linkType: hard
+
 "acorn-walk@npm:^8.1.1, acorn-walk@npm:^8.3.2":
   version: 8.3.2
   resolution: "acorn-walk@npm:8.3.2"
   checksum: 10/57dbe2fd8cf744f562431775741c5c087196cd7a65ce4ccb3f3981cdfad25cd24ad2bad404997b88464ac01e789a0a61e5e355b2a84876f13deef39fb39686ca
   languageName: node
   linkType: hard
 
-"acorn@npm:^8.10.0, acorn@npm:^8.7.1, acorn@npm:^8.8.2, acorn@npm:^8.9.0":
+"acorn@npm:^8.10.0, acorn@npm:^8.11.0, acorn@npm:^8.7.1, acorn@npm:^8.8.2, acorn@npm:^8.9.0":
   version: 8.11.3
   resolution: "acorn@npm:8.11.3"
   bin:
@@ -11832,7 +11842,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"neo-async@npm:^2.6.2":
+"neo-async@npm:^2.6.1, neo-async@npm:^2.6.2":
   version: 2.6.2
   resolution: "neo-async@npm:2.6.2"
   checksum: 10/1a7948fea86f2b33ec766bc899c88796a51ba76a4afc9026764aedc6e7cde692a09067031e4a1bf6db4f978ccd99e7f5b6c03fe47ad9865c3d4f99050d67e002
@@ -13154,6 +13164,21 @@ __metadata:
   languageName: node
   linkType: hard
 
+"react-server-dom-webpack@npm:18.3.0-canary-60a927d04-20240113":
+  version: 18.3.0-canary-60a927d04-20240113
+  resolution: "react-server-dom-webpack@npm:18.3.0-canary-60a927d04-20240113"
+  dependencies:
+    acorn-loose: "npm:^8.3.0"
+    loose-envify: "npm:^1.1.0"
+    neo-async: "npm:^2.6.1"
+  peerDependencies:
+    react: 18.3.0-canary-60a927d04-20240113
+    react-dom: 18.3.0-canary-60a927d04-20240113
+    webpack: ^5.59.0
+  checksum: 10/b52ca5befe8e4470a82b79602d10f9949b90d3a4fd90f2067774ad90b311ecb84bd767ec05e8c64df58b12daf37f8aa09349c41c321d13d062e669d04a91507f
+  languageName: node
+  linkType: hard
+
 "react-style-singleton@npm:^2.2.1":
   version: 2.2.1
   resolution: "react-style-singleton@npm:2.2.1"