Skip to content

Commit 5913346

Browse files
abernixabernix
authored
Sort fragments during operation normalization (De-composed from #1027). (#1112)
* Cutover to apollo-graphql * Add apollo-graphql as a dependency (and project reference) * Remove apollo-engine-reporting as a dependency * Update sortAST to normalize order of fragments w.r.t operations * Tests are failing expectedly at this point * Update tests This update is exactly as expected. All fragments will now move to the beginning of a normalization result. * Use snapshot tests This is a valid use case for snapshots. Even better would be inline snapshots - something we can get to later. * Centralize operation hashing function These two operations will likely be used in tandem, and we want this to be consistent across consumers. * Incorporate rename suggestions * Add explicit comment about sort order * Transform the operation in a less aggressive manner - don't remove aliases and be less strict about removing literals. * Update incorrect reference to renamed module, update related snapshots. * Revert changes to defaultEngineReportingSignature. Apply changes to new function, defaultOperationRegistrySignature. This new function is the effective interim fix, and the current existing function is now left alone. * Add CHANGELOG.md entry for #1112.
1 parent 2741191 commit 5913346

7 files changed

Lines changed: 368 additions & 154 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## Upcoming
44

5+
- `apollo-graphql`
6+
- Change the `sortAST` algorithm to sort fragments at the top-level of the `DocumentNode`, providing a more deterministic normalization of the operation for use by `apollo-engine-reporting` (which consumes this package's `defaultOperationRegistrySignature` function). This will more correctly combine operations for Engine reporting. This also adds a `defaultOperationRegistrySignature` function for use by the `apollo-server-plugin-operation-registry` plugin to eventually consume. [#1112](https://github.com/apollographql/apollo-tooling/pull/1112)
7+
58
## `apollo@2.6.1`, `apollo-env@0.4.0`
69

710
- `apollo@2.6.1`

‎packages/apollo-graphql/src/__tests__/__snapshots__/operationId.test.ts.snap‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`defaultEngineReportingSignature basic test 1`] = `"{user{name}}"`;
4+
5+
exports[`defaultEngineReportingSignature basic test with query 1`] = `"{user{name}}"`;
6+
7+
exports[`defaultEngineReportingSignature basic with operation name 1`] = `"query OpName{user{name}}"`;
8+
9+
exports[`defaultEngineReportingSignature fragment 1`] = `"fragment Bar on User{asd}{user{name...Bar}}"`;
10+
11+
exports[`defaultEngineReportingSignature fragments in various order 1`] = `"fragment Bar on User{asd}{user{name...Bar}}"`;
12+
13+
exports[`defaultEngineReportingSignature full test 1`] = `"fragment Bar on User{age@skip(if:$a)...Nested}fragment Nested on User{blah}query Foo($a:Boolean,$b:Int){user(age:0,name:\\"\\"){name tz...Bar...on User{bee hello}}}"`;
14+
15+
exports[`defaultEngineReportingSignature with various argument types 1`] = `"query OpName($a:[[Boolean!]!],$b:EnumType,$c:Int!){user{name(apple:$a,bag:$b,cat:$c)}}"`;
16+
17+
exports[`defaultEngineReportingSignature with various inline types 1`] = `"query OpName{user{name(apple:[],bag:{},cat:ENUM_VALUE)}}"`;
18+
19+
exports[`defaultOperationRegistrySignature basic test 1`] = `"{user{name}}"`;
20+
21+
exports[`defaultOperationRegistrySignature basic test with query 1`] = `"{user{name}}"`;
22+
23+
exports[`defaultOperationRegistrySignature basic with operation name 1`] = `"query OpName{user{name}}"`;
24+
25+
exports[`defaultOperationRegistrySignature fragment 1`] = `"fragment Bar on User{asd}{user{name...Bar}}"`;
26+
27+
exports[`defaultOperationRegistrySignature fragments in various order 1`] = `"fragment Bar on User{asd}{user{name...Bar}}"`;
28+
29+
exports[`defaultOperationRegistrySignature full test 1`] = `"fragment Bar on User{age@skip(if:$a)...Nested}fragment Nested on User{blah}query Foo($a:Boolean,$b:Int){user(age:0,name:\\"\\"){aliased:name tz...Bar...on User{bee hello}}}"`;
30+
31+
exports[`defaultOperationRegistrySignature with various argument types 1`] = `"query OpName($a:[[Boolean!]!],$b:EnumType,$c:Int!){user{name(apple:$a,bag:$b,cat:$c)}}"`;
32+
33+
exports[`defaultOperationRegistrySignature with various inline types 1`] = `"query OpName{user{name(apple:[[0]],bag:{input:\\"\\"},cat:ENUM_VALUE)}}"`;

‎packages/apollo-graphql/src/__tests__/operationId.test.ts‎

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,285 @@
1+
import { default as gql, disableFragmentWarnings } from "graphql-tag";
2+
import {
3+
defaultEngineReportingSignature,
4+
defaultOperationRegistrySignature
5+
} from "../operationId";
6+
7+
// The gql duplicate fragment warning feature really is just warnings; nothing
8+
// breaks if you turn it off in tests.
9+
disableFragmentWarnings();
10+
11+
describe("defaultEngineReportingSignature", () => {
12+
const cases = [
13+
// Test cases borrowed from optics-agent-js.
14+
{
15+
name: "basic test",
16+
operationName: "",
17+
input: gql`
18+
{
19+
user {
20+
name
21+
}
22+
}
23+
`
24+
},
25+
{
26+
name: "basic test with query",
27+
operationName: "",
28+
input: gql`
29+
query {
30+
user {
31+
name
32+
}
33+
}
34+
`
35+
},
36+
{
37+
name: "basic with operation name",
38+
operationName: "OpName",
39+
input: gql`
40+
query OpName {
41+
user {
42+
name
43+
}
44+
}
45+
`
46+
},
47+
{
48+
name: "with various inline types",
49+
operationName: "OpName",
50+
input: gql`
51+
query OpName {
52+
user {
53+
name(apple: [[10]], cat: ENUM_VALUE, bag: { input: "value" })
54+
}
55+
}
56+
`
57+
},
58+
{
59+
name: "with various argument types",
60+
operationName: "OpName",
61+
input: gql`
62+
query OpName($c: Int!, $a: [[Boolean!]!], $b: EnumType) {
63+
user {
64+
name(apple: $a, cat: $c, bag: $b)
65+
}
66+
}
67+
`
68+
},
69+
{
70+
name: "fragment",
71+
operationName: "",
72+
input: gql`
73+
{
74+
user {
75+
name
76+
...Bar
77+
}
78+
}
79+
80+
fragment Bar on User {
81+
asd
82+
}
83+
84+
fragment Baz on User {
85+
jkl
86+
}
87+
`
88+
},
89+
{
90+
name: "fragments in various order",
91+
operationName: "",
92+
input: gql`
93+
fragment Bar on User {
94+
asd
95+
}
96+
97+
{
98+
user {
99+
name
100+
...Bar
101+
}
102+
}
103+
104+
fragment Baz on User {
105+
jkl
106+
}
107+
`
108+
},
109+
{
110+
name: "full test",
111+
operationName: "Foo",
112+
input: gql`
113+
query Foo($b: Int, $a: Boolean) {
114+
user(name: "hello", age: 5) {
115+
...Bar
116+
... on User {
117+
hello
118+
bee
119+
}
120+
tz
121+
aliased: name
122+
}
123+
}
124+
125+
fragment Baz on User {
126+
asd
127+
}
128+
129+
fragment Bar on User {
130+
age @skip(if: $a)
131+
...Nested
132+
}
133+
134+
fragment Nested on User {
135+
blah
136+
}
137+
`
138+
}
139+
];
140+
cases.forEach(({ name, operationName, input }) => {
141+
test(name, () => {
142+
expect(
143+
defaultEngineReportingSignature(input, operationName)
144+
).toMatchSnapshot();
145+
});
146+
});
147+
});
148+
149+
describe("defaultOperationRegistrySignature", () => {
150+
const cases = [
151+
// Test cases borrowed from optics-agent-js.
152+
{
153+
name: "basic test",
154+
operationName: "",
155+
input: gql`
156+
{
157+
user {
158+
name
159+
}
160+
}
161+
`
162+
},
163+
{
164+
name: "basic test with query",
165+
operationName: "",
166+
input: gql`
167+
query {
168+
user {
169+
name
170+
}
171+
}
172+
`
173+
},
174+
{
175+
name: "basic with operation name",
176+
operationName: "OpName",
177+
input: gql`
178+
query OpName {
179+
user {
180+
name
181+
}
182+
}
183+
`
184+
},
185+
{
186+
name: "with various inline types",
187+
operationName: "OpName",
188+
input: gql`
189+
query OpName {
190+
user {
191+
name(apple: [[10]], cat: ENUM_VALUE, bag: { input: "value" })
192+
}
193+
}
194+
`
195+
},
196+
{
197+
name: "with various argument types",
198+
operationName: "OpName",
199+
input: gql`
200+
query OpName($c: Int!, $a: [[Boolean!]!], $b: EnumType) {
201+
user {
202+
name(apple: $a, cat: $c, bag: $b)
203+
}
204+
}
205+
`
206+
},
207+
{
208+
name: "fragment",
209+
operationName: "",
210+
input: gql`
211+
{
212+
user {
213+
name
214+
...Bar
215+
}
216+
}
217+
218+
fragment Bar on User {
219+
asd
220+
}
221+
222+
fragment Baz on User {
223+
jkl
224+
}
225+
`
226+
},
227+
{
228+
name: "fragments in various order",
229+
operationName: "",
230+
input: gql`
231+
fragment Bar on User {
232+
asd
233+
}
234+
235+
{
236+
user {
237+
name
238+
...Bar
239+
}
240+
}
241+
242+
fragment Baz on User {
243+
jkl
244+
}
245+
`
246+
},
247+
{
248+
name: "full test",
249+
operationName: "Foo",
250+
input: gql`
251+
query Foo($b: Int, $a: Boolean) {
252+
user(name: "hello", age: 5) {
253+
...Bar
254+
... on User {
255+
hello
256+
bee
257+
}
258+
tz
259+
aliased: name
260+
}
261+
}
262+
263+
fragment Baz on User {
264+
asd
265+
}
266+
267+
fragment Bar on User {
268+
age @skip(if: $a)
269+
...Nested
270+
}
271+
272+
fragment Nested on User {
273+
blah
274+
}
275+
`
276+
}
277+
];
278+
cases.forEach(({ name, operationName, input }) => {
279+
test(name, () => {
280+
expect(
281+
defaultOperationRegistrySignature(input, operationName)
282+
).toMatchSnapshot();
283+
});
284+
});
285+
});

0 commit comments

Comments
 (0)
⚡