Skip to content

Commit d694ff4

Browse files
jerelmillerbenjamn
jerelmiller
and
benjamn
authored
More efficient implementation when conditionally stripping typename from variables (#10890)
Co-authored-by: Ben Newman <[email protected]>
1 parent fb95eca commit d694ff4

File tree

7 files changed

+177
-256
lines changed

7 files changed

+177
-256
lines changed

‎src/link/remove-typename/__tests__/removeTypenameFromVariables.ts

+102
Original file line numberDiff line numberDiff line change
@@ -415,3 +415,105 @@ test('handles multiple configured types with fields', async () => {
415415
},
416416
});
417417
});
418+
419+
test('handles when __typename is not present in all paths', async () => {
420+
const query = gql`
421+
query Test($foo: JSON, $bar: BarInput) {
422+
someField(foo: $foo, bar: $bar)
423+
}
424+
`;
425+
426+
const link = removeTypenameFromVariables({
427+
except: {
428+
JSON: KEEP,
429+
},
430+
});
431+
432+
const { variables } = await execute(link, {
433+
query,
434+
variables: {
435+
foo: {
436+
foo: true,
437+
baz: { __typename: 'Baz', baz: true },
438+
},
439+
bar: { bar: true },
440+
qux: { __typename: 'Qux', bar: true },
441+
},
442+
});
443+
444+
expect(variables).toStrictEqual({
445+
foo: {
446+
foo: true,
447+
baz: { __typename: 'Baz', baz: true },
448+
},
449+
bar: { bar: true },
450+
qux: { bar: true },
451+
});
452+
});
453+
454+
test('handles when __typename is not present in variables', async () => {
455+
const query = gql`
456+
query Test($foo: JSON, $bar: BarInput) {
457+
someField(foo: $foo, bar: $bar)
458+
}
459+
`;
460+
461+
const link = removeTypenameFromVariables({
462+
except: {
463+
JSON: KEEP,
464+
},
465+
});
466+
467+
const { variables } = await execute(link, {
468+
query,
469+
variables: {
470+
foo: {
471+
foo: true,
472+
baz: { baz: true },
473+
},
474+
bar: { bar: true },
475+
qux: [{ foo: true }],
476+
},
477+
});
478+
479+
expect(variables).toStrictEqual({
480+
foo: {
481+
foo: true,
482+
baz: { baz: true },
483+
},
484+
bar: { bar: true },
485+
qux: [{ foo: true }],
486+
});
487+
});
488+
489+
test('handles when declared variables are unused', async () => {
490+
const query = gql`
491+
query Test($foo: FooInput, $unused: JSON) {
492+
someField(foo: $foo, bar: $bar)
493+
}
494+
`;
495+
496+
const link = removeTypenameFromVariables({
497+
except: {
498+
JSON: KEEP,
499+
},
500+
});
501+
502+
const { variables } = await execute(link, {
503+
query,
504+
variables: {
505+
foo: {
506+
__typename: 'Foo',
507+
foo: true,
508+
baz: { __typename: 'Bar', baz: true },
509+
},
510+
},
511+
});
512+
513+
expect(variables).toStrictEqual({
514+
foo: {
515+
foo: true,
516+
baz: { baz: true },
517+
},
518+
});
519+
});

‎src/link/remove-typename/removeTypenameFromVariables.ts

+66-52
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { Trie } from '@wry/trie';
21
import { wrap } from 'optimism';
32
import type { DocumentNode, TypeNode } from 'graphql';
43
import { Kind, visit } from 'graphql';
54
import { ApolloLink } from '../core';
6-
import { canUseWeakMap, stripTypename } from '../../utilities';
5+
import { stripTypename, isPlainObject } from '../../utilities';
6+
import type { OperationVariables } from '../../core';
77

88
export const KEEP = '__KEEP';
99

@@ -18,55 +18,83 @@ export interface RemoveTypenameFromVariablesOptions {
1818
export function removeTypenameFromVariables(
1919
options: RemoveTypenameFromVariablesOptions = Object.create(null)
2020
) {
21-
const { except } = options;
22-
23-
const trie = new Trie<typeof stripTypename.BREAK>(
24-
canUseWeakMap,
25-
() => stripTypename.BREAK
26-
);
27-
28-
if (except) {
29-
// Use `lookupArray` to store the path in the `trie` ahead of time. We use
30-
// `peekArray` when actually checking if a path is configured in the trie
31-
// to avoid creating additional lookup paths in the trie.
32-
collectPaths(except, (path) => trie.lookupArray(path));
33-
}
34-
3521
return new ApolloLink((operation, forward) => {
22+
const { except } = options;
3623
const { query, variables } = operation;
3724

3825
if (!variables) {
3926
return forward(operation);
4027
}
4128

42-
if (!except) {
43-
return forward({ ...operation, variables: stripTypename(variables) });
44-
}
45-
46-
const variableDefinitions = getVariableDefinitions(query);
47-
4829
return forward({
4930
...operation,
50-
variables: stripTypename(variables, {
51-
keep: (variablePath) => {
52-
const typename = variableDefinitions[variablePath[0]];
53-
54-
// The path configurations do not include array indexes, so we
55-
// omit them when checking the `trie` for a configured path
56-
const withoutArrayIndexes = variablePath.filter(
57-
(segment) => typeof segment === 'string'
58-
);
59-
60-
// Our path configurations use the typename as the root so we need to
61-
// replace the first segment in the variable path with the typename
62-
// instead of the top-level variable name.
63-
return trie.peekArray([typename, ...withoutArrayIndexes.slice(1)]);
64-
},
65-
}),
31+
variables: except
32+
? maybeStripTypenameUsingConfig(query, variables, except)
33+
: stripTypename(variables),
6634
});
6735
});
6836
}
6937

38+
function maybeStripTypenameUsingConfig(
39+
query: DocumentNode,
40+
variables: OperationVariables,
41+
config: KeepTypenameConfig
42+
) {
43+
const variableDefinitions = getVariableDefinitions(query);
44+
45+
return Object.fromEntries(
46+
Object.entries(variables).map((keyVal) => {
47+
const [key, value] = keyVal;
48+
const typename = variableDefinitions[key];
49+
const typenameConfig = config[typename];
50+
51+
keyVal[1] = typenameConfig
52+
? maybeStripTypename(value, typenameConfig)
53+
: stripTypename(value);
54+
55+
return keyVal;
56+
})
57+
);
58+
}
59+
60+
type JSONPrimitive = string | number | null | boolean;
61+
type JSONValue = JSONPrimitive | JSONValue[] | { [key: string]: JSONValue };
62+
63+
function maybeStripTypename(
64+
value: JSONValue,
65+
config: KeepTypenameConfig[string]
66+
): JSONValue {
67+
if (config === KEEP) {
68+
return value;
69+
}
70+
71+
if (Array.isArray(value)) {
72+
return value.map((item) => maybeStripTypename(item, config));
73+
}
74+
75+
if (isPlainObject(value)) {
76+
const modified: Record<string, any> = {};
77+
78+
Object.keys(value).forEach((key) => {
79+
const child = value[key];
80+
81+
if (key === '__typename') {
82+
return;
83+
}
84+
85+
const fieldConfig = config[key];
86+
87+
modified[key] = fieldConfig
88+
? maybeStripTypename(child, fieldConfig)
89+
: stripTypename(child);
90+
});
91+
92+
return modified;
93+
}
94+
95+
return value;
96+
}
97+
7098
const getVariableDefinitions = wrap((document: DocumentNode) => {
7199
const definitions: Record<string, string> = {};
72100

@@ -89,17 +117,3 @@ function unwrapType(node: TypeNode): string {
89117
return node.name.value;
90118
}
91119
}
92-
93-
function collectPaths(
94-
config: KeepTypenameConfig,
95-
register: (path: string[]) => void,
96-
path: string[] = []
97-
) {
98-
Object.entries(config).forEach(([key, value]) => {
99-
if (value === KEEP) {
100-
return register([...path, key]);
101-
}
102-
103-
collectPaths(value, register, path.concat(key));
104-
});
105-
}

‎src/utilities/common/__tests__/omitDeep.ts

-112
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import equal from '@wry/equality';
21
import { omitDeep } from '../omitDeep';
32

43
test('omits the key from a shallow object', () => {
@@ -136,114 +135,3 @@ test('only considers plain objects and ignores class instances when omitting pro
136135
expect(modifiedThing).toBe(thing);
137136
expect(modifiedThing).toHaveProperty('omit', false);
138137
});
139-
140-
test('allows paths to be conditionally kept with the `keep` option by returning `true`', () => {
141-
const original = {
142-
omit: true,
143-
foo: { omit: false, bar: 'bar' },
144-
omitFirst: [
145-
{ omit: true, foo: 'bar' },
146-
{ omit: false, foo: 'bar' },
147-
],
148-
};
149-
150-
const result = omitDeep(original, 'omit', {
151-
keep: (path) => {
152-
return (
153-
equal(path, ['foo', 'omit']) || equal(path, ['omitFirst', 1, 'omit'])
154-
);
155-
},
156-
});
157-
158-
expect(result).toEqual({
159-
foo: { omit: false, bar: 'bar' },
160-
omitFirst: [{ foo: 'bar' }, { omit: false, foo: 'bar' }],
161-
});
162-
});
163-
164-
test('allows path traversal to be skipped with the `keep` option by returning `BREAK`', () => {
165-
const original = {
166-
omit: true,
167-
foo: {
168-
omit: false,
169-
bar: 'bar',
170-
baz: {
171-
foo: 'bar',
172-
omit: false,
173-
},
174-
},
175-
keepAll: [
176-
{ omit: false, foo: 'bar' },
177-
{ omit: false, foo: 'bar' },
178-
],
179-
keepOne: [
180-
{ omit: false, nested: { omit: false, foo: 'bar' } },
181-
{ omit: true, nested: { omit: true, foo: 'bar' } },
182-
],
183-
};
184-
185-
const result = omitDeep(original, 'omit', {
186-
keep: (path) => {
187-
if (
188-
equal(path, ['foo']) ||
189-
equal(path, ['keepAll']) ||
190-
equal(path, ['keepOne', 0])
191-
) {
192-
return omitDeep.BREAK;
193-
}
194-
},
195-
});
196-
197-
expect(result).toEqual({
198-
foo: { omit: false, bar: 'bar', baz: { foo: 'bar', omit: false } },
199-
keepAll: [
200-
{ omit: false, foo: 'bar' },
201-
{ omit: false, foo: 'bar' },
202-
],
203-
keepOne: [
204-
{ omit: false, nested: { omit: false, foo: 'bar' } },
205-
{ nested: { foo: 'bar' } },
206-
],
207-
});
208-
209-
expect(result.foo).toBe(original.foo);
210-
expect(result.keepAll).toBe(original.keepAll);
211-
expect(result.keepOne[0]).toBe(original.keepOne[0]);
212-
});
213-
214-
test('can mix and match `keep` with `true `and `BREAK`', () => {
215-
const original = {
216-
omit: true,
217-
foo: {
218-
omit: false,
219-
bar: 'bar',
220-
baz: {
221-
foo: 'bar',
222-
omit: false,
223-
},
224-
},
225-
omitFirst: [
226-
{ omit: false, foo: 'bar' },
227-
{ omit: true, foo: 'bar' },
228-
],
229-
};
230-
231-
const result = omitDeep(original, 'omit', {
232-
keep: (path) => {
233-
if (equal(path, ['foo'])) {
234-
return omitDeep.BREAK;
235-
}
236-
237-
if (equal(path, ['omitFirst', 0, 'omit'])) {
238-
return true;
239-
}
240-
},
241-
});
242-
243-
expect(result).toEqual({
244-
foo: { omit: false, bar: 'bar', baz: { foo: 'bar', omit: false } },
245-
omitFirst: [{ omit: false, foo: 'bar' }, { foo: 'bar' }],
246-
});
247-
248-
expect(result.foo).toBe(original.foo);
249-
});

0 commit comments

Comments
 (0)