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

major(graphcache): remove warning and lookup fields by name rather than alias #2616

Merged
merged 7 commits into from
Aug 19, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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/slimy-shrimps-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/exchange-graphcache': patch
---

When comparing selection-sets during `optimistic` mutations we shouldn't look at field-aliases but instead leverage field-names. This also removes the warning for missing fields in optimistic return values
79 changes: 8 additions & 71 deletions exchanges/graphcache/src/cacheExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -556,76 +556,6 @@ describe('data dependencies', () => {
expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(2);
});

it('respects aliases in the optimistic update data that is written', () => {
jest.useFakeTimers();

const mutation = gql`
mutation {
concealed: concealAuthor
}
`;

const mutationData = {
__typename: 'Mutation',
concealed: true,
};

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

jest.spyOn(client, 'reexecuteOperation').mockImplementation(next);

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

const response = jest.fn(
(forwardOp: Operation): OperationResult => {
if (forwardOp.key === 1) {
return { operation: opMutation, data: mutationData };
}

return undefined as any;
}
);

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

const updates = {
Mutation: {
concealAuthor: jest.fn(),
},
};

const optimistic = {
concealAuthor: jest.fn(() => true) as any,
};

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

next(opMutation);
expect(optimistic.concealAuthor).toHaveBeenCalledTimes(1);
expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(1);

const data = updates.Mutation.concealAuthor.mock.calls[0][0];
// Expect both fields to exist
expect(data.concealed).toBe(true);
expect(data.concealAuthor).toBe(true);

jest.runAllTimers();
expect(updates.Mutation.concealAuthor).toHaveBeenCalledTimes(2);
});

it('marks errored null fields as uncached but delivers them as expected', () => {
const client = createClient({ url: 'http://0.0.0.0' });
const { source: ops$, next } = makeSubject<Operation>();
Expand Down Expand Up @@ -708,7 +638,9 @@ describe('optimistic updates', () => {
concealAuthor: {
__typename: 'Author',
id: '123',
name: '[REDACTED OFFLINE]',
name() {
return '[REDACTED OFFLINE]';
},
},
};

Expand Down Expand Up @@ -772,6 +704,11 @@ describe('optimistic updates', () => {
expect(optimistic.concealAuthor).toHaveBeenCalledTimes(1);
expect(reexec).toHaveBeenCalledTimes(1);

expect(result.mock.calls[1][0]?.data).toMatchObject({
__typename: 'Query',
author: { name: '[REDACTED OFFLINE]' },
});

jest.runAllTimers();
expect(response).toHaveBeenCalledTimes(2);
expect(result).toHaveBeenCalledTimes(4);
Expand Down
34 changes: 20 additions & 14 deletions exchanges/graphcache/src/operations/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
OperationRequest,
Dependencies,
EntityField,
OptimisticMutationResolver,
} from '../types';

import {
Expand Down Expand Up @@ -224,15 +225,16 @@ const writeSelection = (
const fieldArgs = getFieldArguments(node, ctx.variables);
const fieldKey = keyOfField(fieldName, fieldArgs);
const fieldAlias = getFieldAlias(node);
let fieldValue = data[fieldAlias];
let fieldValue = data[ctx.optimistic ? fieldName : fieldAlias];

// Development check of undefined fields
if (process.env.NODE_ENV !== 'production') {
if (!isRoot && fieldValue === undefined && !deferRef.current) {
const advice = ctx.optimistic
? '\nYour optimistic result may be missing a field!'
: '';

if (
!isRoot &&
fieldValue === undefined &&
!deferRef.current &&
!ctx.optimistic
) {
const expected =
node.selectionSet === undefined
? 'scalar (number, boolean, etc)'
Expand All @@ -243,8 +245,7 @@ const writeSelection = (
fieldKey +
'` is `undefined`, but the GraphQL query expects a ' +
expected +
' for this field.' +
advice,
' for this field.',
13
);

Expand All @@ -266,16 +267,21 @@ const writeSelection = (
// Add the current alias to the walked path before processing the field's value
ctx.__internal.path.push(fieldAlias);

// Execute optimistic mutation functions on root fields
// Execute optimistic mutation functions on root fields, or execute recursive functions
// that have been returned on optimistic objects
let resolver: OptimisticMutationResolver | void;
if (ctx.optimistic && isRoot) {
const resolver = ctx.store.optimisticMutations[fieldName];

resolver = ctx.store.optimisticMutations[fieldName];
if (!resolver) continue;
} else if (ctx.optimistic && typeof fieldValue === 'function') {
resolver = fieldValue as any;
}

// Execute the field-level resolver to retrieve its data
if (resolver) {
// We have to update the context to reflect up-to-date ResolveInfo
updateContext(ctx, data, typename, typename, fieldKey, fieldName);
fieldValue = data[fieldAlias] = ensureData(
resolver(fieldArgs || {}, ctx.store, ctx)
);
fieldValue = ensureData(resolver(fieldArgs || {}, ctx.store, ctx));
}

if (node.selectionSet) {
Expand Down
18 changes: 17 additions & 1 deletion exchanges/graphcache/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,23 @@ export type UpdatesConfig = {
};
};

export type MakeFunctional<T> = T extends { __typename: string }
? WithTypename<
{
[P in keyof T]?: MakeFunctional<T[P]>;
}
>
: OptimisticMutationResolver<Variables, T> | T;

export type OptimisticMutationResolver<
Args = Variables,
Result = Link<Data> | Scalar
> = {
bivarianceHack(vars: Args, cache: Cache, info: ResolveInfo): Result;
bivarianceHack(
vars: Args,
cache: Cache,
info: ResolveInfo
): MakeFunctional<Result>;
}['bivarianceHack'];

export type OptimisticMutationConfig = {
Expand Down Expand Up @@ -226,3 +238,7 @@ export type Dependencies = Set<string>;

/** The type of cache operation being executed. */
export type OperationType = 'read' | 'write';

export type WithTypename<T extends { __typename?: any }> = T & {
__typename: NonNullable<T['__typename']>;
};