Skip to content

Commit

Permalink
feat(core): Reimplement fetch source as async generator (#3043)
Browse files Browse the repository at this point in the history
  • Loading branch information
kitten authored Mar 15, 2023
1 parent d41fac8 commit 81cf542
Show file tree
Hide file tree
Showing 13 changed files with 214 additions and 255 deletions.
7 changes: 7 additions & 0 deletions .changeset/real-donkeys-act.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@urql/core': patch
---

Replace fetch source implementation with async generator implementation, based on Wonka's `fromAsyncIterable`.
This also further hardens our support for the "Incremental Delivery" specification and
refactors its implementation and covers more edge cases.
1 change: 1 addition & 0 deletions exchanges/execute/src/execute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ describe('on operation', () => {

fetchMock.mockResolvedValue({
status: 200,
headers: { get: () => 'application/json' },
text: vi
.fn()
.mockResolvedValue(JSON.stringify({ data: mockHttpResponseData })),
Expand Down
12 changes: 10 additions & 2 deletions exchanges/graphcache/src/offlineExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ const storage = {

describe('storage', () => {
it('should read the metadata and dispatch operations on initialization', () => {
const client = createClient({ url: 'http://0.0.0.0' });
const client = createClient({
url: 'http://0.0.0.0',
exchanges: [],
});

const reexecuteOperation = vi
.spyOn(client, 'reexecuteOperation')
.mockImplementation(() => undefined);
Expand Down Expand Up @@ -100,7 +104,11 @@ describe('offline', () => {
it('should intercept errored mutations', () => {
const onlineSpy = vi.spyOn(navigator, 'onLine', 'get');

const client = createClient({ url: 'http://0.0.0.0' });
const client = createClient({
url: 'http://0.0.0.0',
exchanges: [],
});

const queryOp = client.createRequestOperation('query', {
key: 1,
query: queryOne,
Expand Down
60 changes: 6 additions & 54 deletions exchanges/multipart-fetch/src/multipartFetchExchange.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Client, OperationResult, makeOperation } from '@urql/core';
import { empty, fromValue, pipe, Source, subscribe, toPromise } from 'wonka';
import { Client, OperationResult } from '@urql/core';
import { empty, fromValue, pipe, Source, toPromise } from 'wonka';

import {
vi,
expect,
Expand All @@ -23,9 +24,6 @@ import {
const fetch = (global as any).fetch as Mock;
const abort = vi.fn();

const abortError = new Error();
abortError.name = 'AbortError';

beforeAll(() => {
(global as any).AbortController = function AbortController() {
this.signal = undefined;
Expand Down Expand Up @@ -61,6 +59,7 @@ describe('on success', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 200,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(response),
});
});
Expand Down Expand Up @@ -130,6 +129,7 @@ describe('on error', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 400,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue('{}'),
});
});
Expand Down Expand Up @@ -165,6 +165,7 @@ describe('on error', () => {
it('ignores the error when a result is available', async () => {
fetch.mockResolvedValue({
status: 400,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(response),
});

Expand All @@ -177,52 +178,3 @@ describe('on error', () => {
expect(data.data).toEqual(JSON.parse(response).data);
});
});

describe('on teardown', () => {
const fail = () => {
expect(true).toEqual(false);
};

it('does not start the outgoing request on immediate teardowns', () => {
fetch.mockRejectedValueOnce(abortError);

const { unsubscribe } = pipe(
fromValue(queryOperation),
multipartFetchExchange(exchangeArgs),
subscribe(fail)
);

unsubscribe();
expect(fetch).toHaveBeenCalledTimes(0);
expect(abort).toHaveBeenCalledTimes(1);
});

it('aborts the outgoing request', async () => {
fetch.mockRejectedValueOnce(abortError);

const { unsubscribe } = pipe(
fromValue(queryOperation),
multipartFetchExchange(exchangeArgs),
subscribe(fail)
);

await Promise.resolve();

unsubscribe();
expect(fetch).toHaveBeenCalledTimes(1);
expect(abort).toHaveBeenCalledTimes(1);
});

it('does not call the query', () => {
pipe(
fromValue(
makeOperation('teardown', queryOperation, queryOperation.context)
),
multipartFetchExchange(exchangeArgs),
subscribe(fail)
);

expect(fetch).toHaveBeenCalledTimes(0);
expect(abort).toHaveBeenCalledTimes(0);
});
});
16 changes: 16 additions & 0 deletions exchanges/persisted-fetch/src/persistedFetchExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ it('accepts successful persisted query responses', async () => {
});

fetch.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expected),
});

Expand Down Expand Up @@ -63,9 +65,13 @@ it('supports cache-miss persisted query errors', async () => {

fetch
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedMiss),
})
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedRetry),
});

Expand Down Expand Up @@ -94,9 +100,13 @@ it('supports GET exclusively for persisted queries', async () => {

fetch
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedMiss),
})
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedRetry),
});

Expand Down Expand Up @@ -127,12 +137,18 @@ it('supports unsupported persisted query errors', async () => {

fetch
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedMiss),
})
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedRetry),
})
.mockResolvedValueOnce({
status: 200,
headers: { get: () => 'application/json' },
text: () => Promise.resolve(expectedRetry),
});

Expand Down
17 changes: 13 additions & 4 deletions packages/core/src/exchanges/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ describe('on success', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 200,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(response),
});
});
Expand Down Expand Up @@ -91,6 +92,7 @@ describe('on error', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 400,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(JSON.stringify({})),
});
});
Expand Down Expand Up @@ -126,6 +128,7 @@ describe('on error', () => {
it('ignores the error when a result is available', async () => {
fetch.mockResolvedValue({
status: 400,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(response),
});

Expand All @@ -143,8 +146,9 @@ describe('on teardown', () => {
const fail = () => {
expect(true).toEqual(false);
};

it('does not start the outgoing request on immediate teardowns', () => {
fetch.mockRejectedValueOnce(abortError);
fetch.mockResolvedValue(new Response('text', { status: 200 }));

const { unsubscribe } = pipe(
fromValue(queryOperation),
Expand All @@ -154,11 +158,11 @@ describe('on teardown', () => {

unsubscribe();
expect(fetch).toHaveBeenCalledTimes(0);
expect(abort).toHaveBeenCalledTimes(1);
expect(abort).toHaveBeenCalledTimes(0);
});

it('aborts the outgoing request', async () => {
fetch.mockRejectedValueOnce(abortError);
fetch.mockResolvedValue(new Response('text', { status: 200 }));

const { unsubscribe } = pipe(
fromValue(queryOperation),
Expand All @@ -169,11 +173,16 @@ describe('on teardown', () => {
await Promise.resolve();

unsubscribe();

// NOTE: We can only observe the async iterator's final run after a macro tick
await new Promise(resolve => setTimeout(resolve));
expect(fetch).toHaveBeenCalledTimes(1);
expect(abort).toHaveBeenCalledTimes(1);
expect(abort).toHaveBeenCalled();
});

it('does not call the query', () => {
fetch.mockResolvedValue(new Response('text', { status: 200 }));

pipe(
fromValue(
makeOperation('teardown', queryOperation, queryOperation.context)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,9 @@ exports[`on success > uses the mock fetch if given 1`] = `
{
"type": "return",
"value": {
"headers": {
"get": [Function],
},
"status": 200,
"text": [MockFunction spy] {
"calls": [
Expand Down
16 changes: 9 additions & 7 deletions packages/core/src/internal/fetchSource.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ import { makeOperation } from '../utils';
const fetch = (global as any).fetch as Mock;
const abort = vi.fn();

const abortError = new Error();
abortError.name = 'AbortError';

beforeAll(() => {
(global as any).AbortController = function AbortController() {
this.signal = undefined;
Expand Down Expand Up @@ -51,6 +48,7 @@ describe('on success', () => {
beforeEach(() => {
fetch.mockResolvedValue({
status: 200,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(response),
});
});
Expand All @@ -73,6 +71,7 @@ describe('on success', () => {
const fetchOptions = {};
const fetcher = vi.fn().mockResolvedValue({
status: 200,
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue(response),
});

Expand Down Expand Up @@ -102,6 +101,7 @@ describe('on error', () => {
fetch.mockResolvedValue({
status: 400,
statusText: 'Forbidden',
headers: { get: () => 'application/json' },
text: vi.fn().mockResolvedValue('{}'),
});
});
Expand Down Expand Up @@ -165,7 +165,7 @@ describe('on teardown', () => {
};

it('does not start the outgoing request on immediate teardowns', () => {
fetch.mockRejectedValue(abortError);
fetch.mockResolvedValue(new Response('text', { status: 200 }));

const { unsubscribe } = pipe(
makeFetchSource(queryOperation, 'https://test.com/graphql', {}),
Expand All @@ -174,20 +174,22 @@ describe('on teardown', () => {

unsubscribe();
expect(fetch).toHaveBeenCalledTimes(0);
expect(abort).toHaveBeenCalledTimes(1);
expect(abort).toHaveBeenCalledTimes(0);
});

it('aborts the outgoing request', async () => {
fetch.mockRejectedValue(abortError);
fetch.mockResolvedValue(new Response('text', { status: 200 }));

const { unsubscribe } = pipe(
makeFetchSource(queryOperation, 'https://test.com/graphql', {}),
subscribe(fail)
);

await Promise.resolve();

unsubscribe();

// NOTE: We can only observe the async iterator's final run after a macro tick
await new Promise(resolve => setTimeout(resolve));
expect(fetch).toHaveBeenCalledTimes(1);
expect(abort).toHaveBeenCalledTimes(1);
});
Expand Down
Loading

0 comments on commit 81cf542

Please sign in to comment.