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

(core) - Deprecate the "operationName" property and move to "kind" #1045

Merged
merged 21 commits into from
Oct 28, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
6 changes: 6 additions & 0 deletions .changeset/fresh-ligers-relax.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@urql/core': patch
'urql': patch
---

Deprecate the "Operation.operationName" property in favor of "Operation.kind"
41 changes: 22 additions & 19 deletions packages/core/src/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import gql from 'graphql-tag';

/** NOTE: Testing in this file is designed to test both the client and its interaction with default Exchanges */

import { map, pipe, subscribe, filter, toArray, tap } from 'wonka';
import { Source, map, pipe, subscribe, filter, toArray, tap } from 'wonka';
import { Exchange, Operation, OperationResult } from './types';
import { createClient } from './client';
import { queryOperation } from './test-utils';
Expand Down Expand Up @@ -32,9 +32,9 @@ const query = {
variables: { example: 1234 },
};

let receivedOps: any[] = [];
let receivedOps: Operation[] = [];
let client = createClient({ url: '1234' });
const receiveMock = jest.fn(s =>
const receiveMock = jest.fn((s: Source<Operation>) =>
pipe(
s,
tap(op => (receivedOps = [...receivedOps, op])),
Expand Down Expand Up @@ -86,7 +86,7 @@ describe('promisified methods', () => {
expect(print(received.query)).toEqual(print(query.query));
expect(received.key).toBeDefined();
expect(received.variables).toEqual({ example: 1234 });
expect(received.operationName).toEqual('query');
expect(received.kind).toEqual('query');
expect(received.context).toEqual({
url: 'https://hostname.com',
requestPolicy: 'cache-only',
Expand Down Expand Up @@ -116,7 +116,7 @@ describe('promisified methods', () => {
expect(print(received.query)).toEqual(print(query.query));
expect(received.key).toBeDefined();
expect(received.variables).toEqual({ example: 1234 });
expect(received.operationName).toEqual('mutation');
expect(received.kind).toEqual('mutation');
expect(received.context).toEqual({
url: 'https://hostname.com',
requestPolicy: 'cache-and-network',
Expand All @@ -142,13 +142,16 @@ describe('synchronous methods', () => {
);

expect(receivedOps.length).toBe(2);
expect(receivedOps[0].operationName).toBe('query');
expect(receivedOps[1].operationName).toBe('teardown');
expect(receivedOps[0].kind).toBe('query');
expect(receivedOps[1].kind).toBe('teardown');
expect(result).toEqual({
operation: {
...query,
context: expect.anything(),
key: expect.any(Number),
kind: 'query',

// TODO: Remove this when the deprecated `operationName` property is removed
operationName: 'query',
},
});
Expand Down Expand Up @@ -199,13 +202,13 @@ describe('executeQuery', () => {
);
});

it('passes operationName type to exchange', () => {
it('passes kind type to exchange', () => {
pipe(
client.executeQuery(query),
subscribe(x => x)
);

expect(receivedOps[0]).toHaveProperty('operationName', 'query');
expect(receivedOps[0]).toHaveProperty('kind', 'query');
});

it('passes url (from context) to exchange', () => {
Expand All @@ -227,11 +230,11 @@ describe('executeQuery', () => {
expect(receivedOps.length).toEqual(1);
jest.advanceTimersByTime(200);
expect(receivedOps.length).toEqual(5);
expect(receivedOps[0].operationName).toEqual('query');
expect(receivedOps[1].operationName).toEqual('teardown');
expect(receivedOps[2].operationName).toEqual('query');
expect(receivedOps[3].operationName).toEqual('teardown');
expect(receivedOps[4].operationName).toEqual('query');
expect(receivedOps[0].kind).toEqual('query');
expect(receivedOps[1].kind).toEqual('teardown');
expect(receivedOps[2].kind).toEqual('query');
expect(receivedOps[3].kind).toEqual('teardown');
expect(receivedOps[4].kind).toEqual('query');
unsubscribe();
});
});
Expand All @@ -256,13 +259,13 @@ describe('executeMutation', () => {
expect(receivedOps[0]).toHaveProperty('variables', query.variables);
});

it('passes operationName type to exchange', () => {
it('passes kind type to exchange', () => {
pipe(
client.executeMutation(query),
subscribe(x => x)
);

expect(receivedOps[0]).toHaveProperty('operationName', 'mutation');
expect(receivedOps[0]).toHaveProperty('kind', 'mutation');
});

it('passes url (from context) to exchange', () => {
Expand Down Expand Up @@ -295,13 +298,13 @@ describe('executeSubscription', () => {
expect(receivedOps[0]).toHaveProperty('variables', query.variables);
});

it('passes operationName type to exchange', () => {
it('passes kind type to exchange', () => {
pipe(
client.executeSubscription(query),
subscribe(x => x)
);

expect(receivedOps[0]).toHaveProperty('operationName', 'subscription');
expect(receivedOps[0]).toHaveProperty('kind', 'subscription');
});
});

Expand All @@ -312,7 +315,7 @@ describe('queuing behavior', () => {
const exchange: Exchange = ({ client }) => ops$ => {
return pipe(
ops$,
filter(op => op.operationName !== 'teardown'),
filter(op => op.kind !== 'teardown'),
tap(op => {
output.push(op);
if (
Expand Down
28 changes: 12 additions & 16 deletions packages/core/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
withPromise,
maskTypename,
noop,
makeOperation,
} from './utils';

import { DocumentNode } from 'graphql';
Expand Down Expand Up @@ -136,7 +137,7 @@ export class Client {
// Reexecute operation only if any subscribers are still subscribed to the
// operation's exchange results
if (
operation.operationName === 'mutation' ||
operation.kind === 'mutation' ||
(this.activeOperations[operation.key] || 0) > 0
) {
this.queue.push(operation);
Expand Down Expand Up @@ -181,16 +182,11 @@ export class Client {
});

createRequestOperation = (
type: OperationType,
kind: OperationType,
request: GraphQLRequest,
opts?: Partial<OperationContext>
): Operation => ({
key: request.key,
query: request.query,
variables: request.variables,
operationName: type,
context: this.createOperationContext(opts),
});
): Operation =>
makeOperation(kind, request, this.createOperationContext(opts));

/** Counts up the active operation key and dispatches the operation */
private onOperationStart(operation: Operation) {
Expand All @@ -207,13 +203,15 @@ export class Client {
prevActive <= 0 ? 0 : prevActive - 1);

if (newActive <= 0) {
this.dispatchOperation({ ...operation, operationName: 'teardown' });
this.dispatchOperation(
makeOperation('teardown', operation, operation.context)
);
}
}

/** Executes an Operation by sending it through the exchange pipeline It returns an observable that emits all related exchange results and keeps track of this observable's subscribers. A teardown signal will be emitted when no subscribers are listening anymore. */
executeRequestOperation(operation: Operation): Source<OperationResult> {
const { key, operationName } = operation;
const { key, kind } = operation;
let operationResults$ = pipe(
this.results$,
filter((res: OperationResult) => res.operation.key === key)
Expand All @@ -229,7 +227,7 @@ export class Client {
);
}

if (operationName === 'mutation') {
if (kind === 'mutation') {
// A mutation is always limited to just a single result and is never shared
return pipe(
operationResults$,
Expand All @@ -240,9 +238,7 @@ export class Client {

const teardown$ = pipe(
this.operations$,
filter(
(op: Operation) => op.operationName === 'teardown' && op.key === key
)
filter((op: Operation) => op.kind === 'teardown' && op.key === key)
);

const result$ = pipe(
Expand All @@ -258,7 +254,7 @@ export class Client {

return operation.context.suspense !== false &&
this.suspense &&
operationName === 'query'
kind === 'query'
? toSuspenseSource<OperationResult>(result$ as Source<OperationResult>)
: (result$ as Source<OperationResult>);
}
Expand Down
3 changes: 3 additions & 0 deletions packages/core/src/exchanges/__snapshots__/fetch.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Object {
"url": "http://localhost:3000/graphql",
},
"key": 2,
"kind": "query",
"operationName": "query",
"query": Object {
"definitions": Array [
Expand Down Expand Up @@ -153,6 +154,7 @@ Object {
"url": "http://localhost:3000/graphql",
},
"key": 2,
"kind": "query",
"operationName": "query",
"query": Object {
"definitions": Array [
Expand Down Expand Up @@ -294,6 +296,7 @@ Object {
"url": "http://localhost:3000/graphql",
},
"key": 2,
"kind": "query",
"operationName": "query",
"query": Object {
"definitions": Array [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Object {
"url": "http://localhost:3000/graphql",
},
"key": 4,
"kind": "subscription",
"operationName": "subscription",
"query": Object {
"definitions": Array [
Expand Down
24 changes: 8 additions & 16 deletions packages/core/src/exchanges/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ interface OperationCache {
[key: string]: Set<number>;
}

const shouldSkip = ({ operationName }: Operation) =>
operationName !== 'mutation' && operationName !== 'query';
const shouldSkip = ({ kind }: Operation) =>
kind !== 'mutation' && kind !== 'query';

export const cacheExchange: Exchange = ({ forward, client, dispatchDebug }) => {
const resultCache = new Map() as ResultCache;
Expand All @@ -37,14 +37,14 @@ export const cacheExchange: Exchange = ({ forward, client, dispatchDebug }) => {

const handleAfterQuery = afterQuery(resultCache, operationCache);

const isOperationCached = operation => {
const isOperationCached = (operation: Operation) => {
const {
key,
operationName,
kind,
context: { requestPolicy },
} = operation;
return (
operationName === 'query' &&
kind === 'query' &&
requestPolicy !== 'network-only' &&
(requestPolicy === 'cache-only' || resultCache.has(key))
);
Expand Down Expand Up @@ -102,21 +102,13 @@ export const cacheExchange: Exchange = ({ forward, client, dispatchDebug }) => {
]),
map(op => addMetadata(op, { cacheOutcome: 'miss' })),
filter(
op =>
op.operationName !== 'query' ||
op.context.requestPolicy !== 'cache-only'
op => op.kind !== 'query' || op.context.requestPolicy !== 'cache-only'
),
forward,
tap(response => {
if (
response.operation &&
response.operation.operationName === 'mutation'
) {
if (response.operation && response.operation.kind === 'mutation') {
handleAfterMutation(response);
} else if (
response.operation &&
response.operation.operationName === 'query'
) {
} else if (response.operation && response.operation.kind === 'query') {
handleAfterQuery(response);
}
})
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/exchanges/dedup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '../test-utils';
import { Operation } from '../types';
import { dedupExchange } from './dedup';
import { makeOperation } from '../utils';

const dispatchDebug = jest.fn();
let shouldRespond = false;
Expand Down Expand Up @@ -82,7 +83,7 @@ it('forwards duplicate query operations after one was torn down', async () => {

publish(exchange);
next(queryOperation);
next({ ...queryOperation, operationName: 'teardown' });
next(makeOperation('teardown', queryOperation, queryOperation.context));
next(queryOperation);
complete();
expect(forwardedOperations.length).toBe(3);
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/exchanges/dedup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ export const dedupExchange: Exchange = ({ forward, dispatchDebug }) => {
const inFlightKeys = new Set<number>();

const filterIncomingOperation = (operation: Operation) => {
const { key, operationName } = operation;
if (operationName === 'teardown') {
const { key, kind } = operation;
if (kind === 'teardown') {
inFlightKeys.delete(key);
return true;
}

if (operationName !== 'query' && operationName !== 'subscription') {
if (kind !== 'query' && kind !== 'subscription') {
return true;
}

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/exchanges/fallback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ export const fallbackExchange: ({
ops$,
tap<Operation>(operation => {
if (
operation.operationName !== 'teardown' &&
operation.kind !== 'teardown' &&
process.env.NODE_ENV !== 'production'
) {
const message = `No exchange has handled operations of type "${operation.operationName}". Check whether you've added an exchange responsible for these operations.`;
const message = `No exchange has handled operations of kind "${operation.kind}". Check whether you've added an exchange responsible for these operations.`;

dispatchDebug({
type: 'fallbackCatch',
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/exchanges/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { empty, fromValue, pipe, Source, subscribe, toPromise } from 'wonka';

import { Client } from '../client';
import { makeOperation } from '../utils';
import { queryOperation } from '../test-utils';
import { OperationResult, OperationType } from '../types';
import { OperationResult } from '../types';
import { fetchExchange } from './fetch';

const fetch = (global as any).fetch as jest.Mock;
Expand Down Expand Up @@ -160,10 +161,9 @@ describe('on teardown', () => {

it('does not call the query', () => {
pipe(
fromValue({
...queryOperation,
operationName: 'teardown' as OperationType,
}),
fromValue(
makeOperation('teardown', queryOperation, queryOperation.context)
),
fetchExchange(exchangeArgs),
subscribe(fail)
);
Expand Down
Loading