Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): Force client.reexecuteOperation to dispatch #3363

Merged
merged 3 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/wicked-seahorses-smash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/core': patch
---

Explicitly unblock `client.reexecuteOperation` calls to allow stalled operations from continuing and re-executing. Previously, this could cause `@urql/exchange-graphcache` to stall if an optimistic mutation led to a cache miss.
139 changes: 139 additions & 0 deletions exchanges/graphcache/src/cacheExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1257,6 +1257,145 @@ describe('optimistic updates', () => {
expect(updates.Mutation.addAuthor).toHaveBeenCalledTimes(2);
expect(response).toHaveBeenCalledTimes(2);
expect(result).toHaveBeenCalledTimes(4);
expect(reexec).toHaveBeenCalledTimes(2);

next(opOne);
vi.runAllTimers();
expect(result).toHaveBeenCalledTimes(5);
});

it('does not block subsequent query operations', () => {
vi.useFakeTimers();

const authorsQuery = gql`
query {
authors {
id
name
}
}
`;

const authorsQueryData = {
__typename: 'Query',
authors: [
{
__typename: 'Author',
id: '123',
name: 'Author',
},
],
};

const mutation = gql`
mutation {
deleteAuthor {
id
name
}
}
`;

const optimisticMutationData = {
__typename: 'Mutation',
deleteAuthor: {
__typename: 'Author',
id: '123',
name: '[REDACTED OFFLINE]',
},
};

const client = createClient({
url: 'http://0.0.0.0',
exchanges: [],
});
const { source: ops$, next } = makeSubject<Operation>();

const reexec = vi
.spyOn(client, 'reexecuteOperation')
.mockImplementation(next);

const opOne = client.createRequestOperation('query', {
key: 1,
query: authorsQuery,
variables: undefined,
});

const opMutation = client.createRequestOperation('mutation', {
key: 2,
query: mutation,
variables: undefined,
});

const response = vi.fn((forwardOp: Operation): OperationResult => {
if (forwardOp.key === 1) {
return { ...queryResponse, operation: opOne, data: authorsQueryData };
} else if (forwardOp.key === 2) {
return {
...queryResponse,
operation: opMutation,
data: {
__typename: 'Mutation',
deleteAuthor: optimisticMutationData.deleteAuthor,
},
};
}

return undefined as any;
});

const result = vi.fn();
const forward: ExchangeIO = ops$ =>
pipe(ops$, delay(1), map(response), share);

const optimistic = {
deleteAuthor: vi.fn(() => optimisticMutationData.deleteAuthor) as any,
};

const updates = {
Mutation: {
deleteAuthor: vi.fn((_data, _, cache) => {
cache.invalidate({
__typename: 'Author',
id: optimisticMutationData.deleteAuthor.id,
});
}),
},
};

pipe(
cacheExchange({ optimistic, updates })({
forward,
client,
dispatchDebug,
})(ops$),
tap(result),
publish
);

next(opOne);
vi.runAllTimers();
expect(response).toHaveBeenCalledTimes(1);
expect(result).toHaveBeenCalledTimes(1);

next(opMutation);
expect(response).toHaveBeenCalledTimes(1);
expect(optimistic.deleteAuthor).toHaveBeenCalledTimes(1);
expect(updates.Mutation.deleteAuthor).toHaveBeenCalledTimes(1);
expect(reexec).toHaveBeenCalledTimes(1);
expect(result).toHaveBeenCalledTimes(1);

vi.runAllTimers();

expect(updates.Mutation.deleteAuthor).toHaveBeenCalledTimes(2);
expect(response).toHaveBeenCalledTimes(2);
expect(result).toHaveBeenCalledTimes(2);
expect(reexec).toHaveBeenCalledTimes(2);
expect(reexec.mock.calls[1][0]).toMatchObject(opOne);

next(opOne);
vi.runAllTimers();
expect(result).toHaveBeenCalledTimes(3);
});
});

Expand Down
3 changes: 2 additions & 1 deletion exchanges/graphcache/src/store/data.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('garbage collection', () => {
it('erases orphaned entities', () => {
InMemoryData.writeRecord('Todo:1', '__typename', 'Todo');
InMemoryData.writeRecord('Todo:1', 'id', '1');
InMemoryData.writeRecord('Todo:2', '__typename', 'Todo');
InMemoryData.writeRecord('Query', '__typename', 'Query');
InMemoryData.writeLink('Query', 'todo', 'Todo:1');

Expand All @@ -27,7 +28,7 @@ describe('garbage collection', () => {
expect(InMemoryData.readRecord('Todo:1', 'id')).toBe(undefined);

expect(InMemoryData.getCurrentDependencies()).toEqual(
new Set(['Todo:1', 'Query.todo'])
new Set(['Todo:1', 'Todo:2', 'Query.todo'])
);
});

Expand Down
40 changes: 40 additions & 0 deletions packages/core/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,46 @@ describe('deduplication behavior', () => {

expect(onOperation).toHaveBeenCalledTimes(2);
});

it('unblocks operations on call to reexecuteOperation', async () => {
const onOperation = vi.fn();
const onResult = vi.fn();

let hasSent = false;
const exchange: Exchange = () => ops$ =>
pipe(
ops$,
onPush(onOperation),
map(op => ({
hasNext: false,
stale: false,
data: 'test',
operation: op,
})),
filter(() => hasSent || !(hasSent = true))
);

const client = createClient({
url: 'test',
exchanges: [exchange],
});

const operation = makeOperation('query', queryOperation, {
...queryOperation.context,
requestPolicy: 'cache-first',
});

pipe(client.executeRequestOperation(operation), subscribe(onResult));

expect(onOperation).toHaveBeenCalledTimes(1);
expect(onResult).toHaveBeenCalledTimes(0);

client.reexecuteOperation(operation);
await Promise.resolve();

expect(onOperation).toHaveBeenCalledTimes(2);
expect(onResult).toHaveBeenCalledTimes(1);
});
});

describe('shared sources behavior', () => {
Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,10 @@ export const Client: new (opts: ClientOptions) => Client = function Client(
if (operation.kind === 'teardown') {
dispatchOperation(operation);
} else if (operation.kind === 'mutation' || active.has(operation.key)) {
let queued = false;
for (let i = 0; i < queue.length; i++)
queued = queued || queue[i].key === operation.key;
if (!queued) dispatched.delete(operation.key);
queue.push(operation);
Promise.resolve().then(dispatchOperation);
}
Expand Down