Repository: urql-graphql/urql
Pull request #3195 (2023-04-28 14:45:54)

Fix re-rendering when another component reuses the same query
<!--
  Thanks for opening a pull request! We appreciate your dedication and help!
  Before submitting your pull request, please make sure to read our CONTRIBUTING guide.

  The best contribution is always a PR, but please make sure to open an issue or discuss
  your changes first, if you’re looking to submit a larger PR.

  If this PR is already related to an issue, please reference it like so:
  Resolves #123
-->

## Summary
When component A uses `useQuery` and has already loaded a result and then component B mounts and uses the same query, it currently causes component A to re-render because `operation` is not shallowly equal even though nothing about the result is changing.

This is a performance issue because it causes unnecessary re-renders.

It also can cause an infinite render loop with lazily loaded components because it can trigger a state update further up the tree during the initial render of the lazily loaded component which makes React back out of the render (which happened in my application, although I've had trouble creating a minimal reproduction). This issue is resolved with this change in my application.
<!-- What's the motivation of this change? What does it solve? -->

## Set of changes
- Added a special case to `isShallowDifferent` for `operation` where it checks the operation key if the `operation`s are not equal
- Fixed a typo in `CONTRIBUTING.md`
<!--
  Roughly list the changes you've made and which packages are affected.
  Leave some notes on what may be noteworthy files you've changed.
  And lastly, please let us know if you think this is a breaking change.
-->



Patch:
diff --git a/.changeset/fair-eels-march.md b/.changeset/fair-eels-march.md
new file mode 100644
index 00000000..7cebb0d1
--- /dev/null
+++ b/.changeset/fair-eels-march.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-persisted': major
+---
+
+Update the `preferGetForPersistedQueries` option to include the `'force'` and `'within-url-limit'` values from the Client's `preferGetMethod` option. The default value of `true` no longer sets `OperationContext`'s `preferGetMethod` setting to `'force'`. Instead, the value of `preferGetForPersistedQueries` carries through to the `OperationContext`'s `preferGetMethod` setting for persisted queries.
diff --git a/.changeset/fifty-sheep-battle.md b/.changeset/fifty-sheep-battle.md
new file mode 100644
index 00000000..30fab153
--- /dev/null
+++ b/.changeset/fifty-sheep-battle.md
@@ -0,0 +1,5 @@
+---
+'urql': patch
+---
+
+Avoid unnecessary re-render when two components use the same query but receive unchanging results, due to differing operations
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index e263eb7d..6d47c40a 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -75,7 +75,7 @@ There are multiple commands you can run in the root folder to test your changes:
 
 ```sh
 # TypeScript checks:
-pnpnm run check
+pnpm run check
 
 # Linting (prettier & eslint):
 pnpm run lint
diff --git a/docs/advanced/persistence-and-uploads.md b/docs/advanced/persistence-and-uploads.md
index e03263e4..f076d07e 100644
--- a/docs/advanced/persistence-and-uploads.md
+++ b/docs/advanced/persistence-and-uploads.md
@@ -61,10 +61,10 @@ const client = new Client({
 });
 ```
 
-As we can see, typically it's recommended to set `preferGetForPersistedQueries` to `true` to force
-all persisted queries to use GET requests instead of POST so that CDNs can do their job.
-It does so by setting the `preferGetMethod` option to `'force'` when it's
-updating operations.
+As we can see, typically it's recommended to set `preferGetForPersistedQueries` to `true`
+to encourage persisted queries to use GET requests instead of POST so that CDNs can do their job.
+When set to `true` or `'within-url-limit'`, persisted queries will use GET requests if the
+resulting URL doesn't exceed the 2048 character limit.
 
 The `fetchExchange` can see the modifications that the `persistedExchange` is
 making to operations, and understands to leave out the `query` from any request
diff --git a/exchanges/persisted/src/persistedExchange.ts b/exchanges/persisted/src/persistedExchange.ts
index cd4d99e8..013a81b5 100644
--- a/exchanges/persisted/src/persistedExchange.ts
+++ b/exchanges/persisted/src/persistedExchange.ts
@@ -17,6 +17,7 @@ import {
   CombinedError,
   Exchange,
   Operation,
+  OperationContext,
 } from '@urql/core';
 
 import { hash } from './sha256';
@@ -29,19 +30,23 @@ const isPersistedUnsupported = (error: CombinedError): boolean =>
 
 /** Input parameters for the {@link persistedExchange}. */
 export interface PersistedExchangeOptions {
-  /** Enforces GET method requests to be made for Persisted Queries.
+  /** Controls whether GET method requests will be made for Persisted Queries.
    *
    * @remarks
-   * When enabled, the `persistedExchange` will set
+   * When set to `true` or `'within-url-limit'`, the `persistedExchange`
+   * will use GET requests on persisted queries when the request URL
+   * doesn't exceed the 2048 character limit.
+   *
+   * When set to `force`, the `persistedExchange` will set
    * `OperationContext.preferGetMethod` to `'force'` on persisted queries,
    * which will force requests to be made using a GET request.
    *
-   * This is frequently used to make GraphQL requests more cacheable
-   * on CDNs.
+   * GET requests are frequently used to make GraphQL requests more
+   * cacheable on CDNs.
    *
-   * @defaultValue `true` - enabled
+   * @defaultValue `undefined` - disabled
    */
-  preferGetForPersistedQueries?: boolean;
+  preferGetForPersistedQueries?: OperationContext['preferGetMethod'];
   /** Enforces non-automatic persisted queries by ignoring APQ errors.
    *
    * @remarks
@@ -118,7 +123,7 @@ export const persistedExchange =
   ({ forward }) => {
     if (!options) options = {};
 
-    const preferGetForPersistedQueries = !!options.preferGetForPersistedQueries;
+    const preferGetForPersistedQueries = options.preferGetForPersistedQueries;
     const enforcePersistedQueries = !!options.enforcePersistedQueries;
     const hashFn = options.generateHash || hash;
     const enableForMutation = !!options.enableForMutation;
@@ -163,7 +168,8 @@ export const persistedExchange =
               persistedOperation.kind === 'query' &&
               preferGetForPersistedQueries
             ) {
-              persistedOperation.context.preferGetMethod = 'force';
+              persistedOperation.context.preferGetMethod =
+                preferGetForPersistedQueries;
             }
           }
 
diff --git a/packages/react-urql/src/hooks/state.ts b/packages/react-urql/src/hooks/state.ts
index c145b477..bf8d4f4a 100644
--- a/packages/react-urql/src/hooks/state.ts
+++ b/packages/react-urql/src/hooks/state.ts
@@ -9,10 +9,29 @@ export const initialState = {
   operation: undefined,
 };
 
-const isShallowDifferent = (a: any, b: any) => {
-  if (typeof a != 'object' || typeof b != 'object') return a !== b;
-  for (const x in a) if (!(x in b)) return true;
-  for (const x in b) if (a[x] !== b[x]) return true;
+// Two operations are considered equal if they have the same key
+const areOperationsEqual = (
+  a: { key: number } | undefined,
+  b: { key: number } | undefined
+) => {
+  return a === b || !!(a && b && a.key === b.key);
+};
+
+/**
+ * Checks if two objects are shallowly different with a special case for
+ * 'operation' where it compares the key if they are not the otherwise equal
+ */
+const isShallowDifferent = <T extends Record<string, any>>(a: T, b: T) => {
+  for (const key in a) if (!(key in b)) return true;
+  for (const key in b) {
+    if (
+      key === 'operation'
+        ? !areOperationsEqual(a[key], b[key])
+        : a[key] !== b[key]
+    ) {
+      return true;
+    }
+  }
   return false;
 };
 
@@ -27,7 +46,7 @@ export const computeNextState = <T extends Stateish>(
   prevState: T,
   result: Partial<T>
 ): T => {
-  const newState = {
+  const newState: T = {
     ...prevState,
     ...result,
     data:


Test patch:
diff --git a/exchanges/persisted/src/persistedExchange.test.ts b/exchanges/persisted/src/persistedExchange.test.ts
index 97dd21e5..8e013801 100644
--- a/exchanges/persisted/src/persistedExchange.test.ts
+++ b/exchanges/persisted/src/persistedExchange.test.ts
@@ -118,3 +118,21 @@ it('retries query persisted query resulted in unsupported', async () => {
 
   expect(operations[2].extensions).toEqual(undefined);
 });
+
+it.each([true, 'force', 'within-url-limit'] as const)(
+  'sets `context.preferGetMethod` to %s when `options.preferGetForPersistedQueries` is %s',
+  async preferGetMethodValue => {
+    const { exchangeArgs } = makeExchangeArgs();
+
+    const res = await pipe(
+      fromValue(queryOperation),
+      persistedExchange({ preferGetForPersistedQueries: preferGetMethodValue })(
+        exchangeArgs
+      ),
+      take(1),
+      toPromise
+    );
+
+    expect(res.operation.context.preferGetMethod).toBe(preferGetMethodValue);
+  }
+);