Skip to content

Commit

Permalink
fix(core): Fix OperationContext._identity not allowing operation cont…
Browse files Browse the repository at this point in the history
…exts to be deeply cloned (#2732)

* fix(core): Fix OperationContext._identity not allowing to be deeply cloned

* Update offlineExchange to remove incoming mutation handling

* Update changeset

* add back assertion

Co-authored-by: Jovi De Croock <[email protected]>
  • Loading branch information
kitten and JoviDeCroock authored Oct 14, 2022
1 parent 976055a commit 3dd23be
Show file tree
Hide file tree
Showing 5 changed files with 34 additions and 28 deletions.
7 changes: 7 additions & 0 deletions .changeset/stupid-rules-mix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@urql/core': patch
'@urql/exchange-graphcache': patch
---

Fix operation identities preventing users from deeply cloning operation contexts. Instead, we now use a client-wide counter (rolling over as needed).
While this changes an internal data structure in `@urql/core` only, this change also affects the `offlineExchange` in `@urql/exchange-graphcache` due to it relying on the identity being previously an object rather than an integer.
20 changes: 13 additions & 7 deletions exchanges/graphcache/src/offlineExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
Operation,
OperationResult,
} from '@urql/core';
import { print } from 'graphql';

import { pipe, map, makeSubject, tap, publish } from 'wonka';
import { offlineExchange } from './offlineExchange';
Expand Down Expand Up @@ -159,6 +160,7 @@ describe('offline', () => {
updateAuthor {
id
name
__typename
}
}`,
variables: {},
Expand Down Expand Up @@ -279,6 +281,7 @@ describe('offline', () => {
updateAuthor {
id
name
__typename
}
}`,
variables: {},
Expand All @@ -288,13 +291,16 @@ describe('offline', () => {
flush!();
expect(reexecuteOperation).toHaveBeenCalledTimes(1);
expect((reexecuteOperation.mock.calls[0][0] as any).key).toEqual(1);
expect((reexecuteOperation.mock.calls[0][0] as any).query).toEqual(gql`
mutation {
updateAuthor {
id
name
expect(print((reexecuteOperation.mock.calls[0][0] as any).query)).toEqual(
print(gql`
mutation {
updateAuthor {
id
name
__typename
}
}
}
`);
`)
);
});
});
22 changes: 3 additions & 19 deletions exchanges/graphcache/src/offlineExchange.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { pipe, merge, makeSubject, share, filter, tap } from 'wonka';
import { pipe, merge, makeSubject, share, filter } from 'wonka';
import { print, SelectionNode } from 'graphql';

import {
Expand Down Expand Up @@ -124,18 +124,11 @@ export const offlineExchange = <C extends Partial<CacheExchangeOpts>>(
isOfflineError(res.error) &&
isOptimisticMutation(optimisticMutations, res.operation)
) {
failedQueue.push(
incomingMutations.get(res.operation.context._instance as []) ||
res.operation
);
failedQueue.push(res.operation);
updateMetadata();
return false;
}

if (res.operation.kind === 'mutation' && !res.error) {
incomingMutations.delete(res.operation.context._instance as []);
}

return true;
})
);
Expand Down Expand Up @@ -171,17 +164,8 @@ export const offlineExchange = <C extends Partial<CacheExchangeOpts>>(
forward,
});

const incomingMutations = new WeakMap<[], Operation>();
return ops$ => {
const sharedOps$ = pipe(
ops$,
tap(operation => {
if (operation.kind === 'mutation') {
incomingMutations.set(operation.context._instance as [], operation);
}
}),
share
);
const sharedOps$ = pipe(ops$, share);

const opsAndRebound$ = merge([reboundOps$, sharedOps$]);

Expand Down
8 changes: 7 additions & 1 deletion packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
ExchangeInput,
GraphQLRequest,
Operation,
OperationInstance,
OperationContext,
OperationResult,
OperationType,
Expand Down Expand Up @@ -149,6 +150,8 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
throw new Error('You are creating an urql-client without a url.');
}

let ids = 0;

const replays = new Map<number, OperationResult>();
const active: Map<number, Source<OperationResult>> = new Map();
const queue: Operation[] = [];
Expand Down Expand Up @@ -286,7 +289,10 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
);
}
return makeOperation(kind, request, {
_instance: kind === 'mutation' ? [] : undefined,
_instance:
kind === 'mutation'
? ((ids = (ids + 1) | 0) as OperationInstance)
: undefined,
...baseOpts,
...opts,
requestPolicy: opts.requestPolicy || baseOpts.requestPolicy,
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,13 @@ export interface OperationDebugMeta {
startTime?: number;
}

/** A unique marker that marks the operation's identity when multiple mutation operations with identical keys are issued. */
export type OperationInstance = number & { readonly _opaque: unique symbol };

/** Additional metadata passed to [exchange]{@link Exchange} functions. */
export interface OperationContext {
[key: string]: any;
readonly _instance?: [] | undefined;
readonly _instance?: OperationInstance | undefined;
additionalTypenames?: string[];
fetch?: typeof fetch;
fetchOptions?: RequestInit | (() => RequestInit);
Expand Down

0 comments on commit 3dd23be

Please sign in to comment.