Skip to content

Commit ab9209f

Browse files
lucastrabalucastraba
authored
fix: Parenthesize implementing type ternaries joined with || (#201)
- Wraps each implementing type expression in parentheses before joining with `||` in the `case 'implement':` codepath (`src/index.ts`), fixing incorrect operator precedence between `||` and `?:` - Updates existing test assertions in `useImplementingTypes`, `useImplementingTypesAndDefaultNullableToNull`, and `useImplementingTypesAndTerminateCircularRelationships` to expect parenthesized output - Adds a new test case with 3+ implementing types to explicitly verify parenthesization
1 parent 743f9cd commit ab9209f

8 files changed

Lines changed: 124 additions & 18 deletions

File tree

‎src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,7 @@ const getNamedType = (opts: Options<NamedTypeNode | ObjectTypeDefinitionNode>):
404404
}),
405405
)
406406
.filter((value) => value !== null)
407+
.map((v) => `(${v})`)
407408
.join(' || ') || null
408409
);
409410
default:

‎tests/useImplementingTypes/__snapshots__/spec.ts.snap‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ export const mockA = (overrides?: Partial<A>): A => {
2525
id: overrides && overrides.hasOwnProperty('id') ? overrides.id! : 'de4b005e-2b2d-4843-94d1-d356d75d933b',
2626
str: overrides && overrides.hasOwnProperty('str') ? overrides.str! : 'cuius',
2727
obj: overrides && overrides.hasOwnProperty('obj') ? overrides.obj! : mockB(),
28-
config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : mockTestAConfig() || mockTestTwoAConfig(),
29-
configArray: overrides && overrides.hasOwnProperty('configArray') ? overrides.configArray! : [mockTestAConfig() || mockTestTwoAConfig()],
30-
field: overrides && overrides.hasOwnProperty('field') ? overrides.field! : mockTestTwoAConfig(),
31-
action: overrides && overrides.hasOwnProperty('action') ? overrides.action! : mockTestAction(),
28+
config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : (mockTestAConfig()) || (mockTestTwoAConfig()),
29+
configArray: overrides && overrides.hasOwnProperty('configArray') ? overrides.configArray! : [(mockTestAConfig()) || (mockTestTwoAConfig())],
30+
field: overrides && overrides.hasOwnProperty('field') ? overrides.field! : (mockTestTwoAConfig()),
31+
action: overrides && overrides.hasOwnProperty('action') ? overrides.action! : (mockTestAction()),
3232
};
3333
};
3434
@@ -152,9 +152,9 @@ export const mockA = (overrides?: Partial<A>): A => {
152152
str: overrides && overrides.hasOwnProperty('str') ? overrides.str! : 'cuius',
153153
obj: overrides && overrides.hasOwnProperty('obj') ? overrides.obj! : mockB(),
154154
config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : 'Karen.Prosacco@gmail.com',
155-
configArray: overrides && overrides.hasOwnProperty('configArray') ? overrides.configArray! : [mockTestAConfig() || mockTestTwoAConfig()],
156-
field: overrides && overrides.hasOwnProperty('field') ? overrides.field! : mockTestTwoAConfig(),
157-
action: overrides && overrides.hasOwnProperty('action') ? overrides.action! : mockTestAction(),
155+
configArray: overrides && overrides.hasOwnProperty('configArray') ? overrides.configArray! : [(mockTestAConfig()) || (mockTestTwoAConfig())],
156+
field: overrides && overrides.hasOwnProperty('field') ? overrides.field! : (mockTestTwoAConfig()),
157+
action: overrides && overrides.hasOwnProperty('action') ? overrides.action! : (mockTestAction()),
158158
};
159159
};
160160

‎tests/useImplementingTypes/spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,19 @@ it('should support useImplementingTypes', async () => {
77
expect(result).toBeDefined();
88

99
expect(result).toContain(
10-
"config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : mockTestAConfig() || mockTestTwoAConfig(),",
10+
"config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : (mockTestAConfig()) || (mockTestTwoAConfig()),",
1111
);
1212

1313
expect(result).toContain(
14-
"configArray: overrides && overrides.hasOwnProperty('configArray') ? overrides.configArray! : [mockTestAConfig() || mockTestTwoAConfig()],",
14+
"configArray: overrides && overrides.hasOwnProperty('configArray') ? overrides.configArray! : [(mockTestAConfig()) || (mockTestTwoAConfig())],",
1515
);
1616

1717
expect(result).toContain(
18-
"field: overrides && overrides.hasOwnProperty('field') ? overrides.field! : mockTestTwoAConfig(),",
18+
"field: overrides && overrides.hasOwnProperty('field') ? overrides.field! : (mockTestTwoAConfig()),",
1919
);
2020

2121
expect(result).toContain(
22-
"action: overrides && overrides.hasOwnProperty('action') ? overrides.action! : mockTestAction(),",
22+
"action: overrides && overrides.hasOwnProperty('action') ? overrides.action! : (mockTestAction()),",
2323
);
2424
expect(result).toMatchSnapshot();
2525
});
@@ -51,7 +51,7 @@ it(`support useImplementingTypes with fieldGeneration prop`, async () => {
5151
);
5252

5353
expect(result).toContain(
54-
"config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : mockTestAConfig() || mockTestTwoAConfig(),",
54+
"config: overrides && overrides.hasOwnProperty('config') ? overrides.config! : (mockTestAConfig()) || (mockTestTwoAConfig()),",
5555
);
5656

5757
result = await plugin(testSchema, [], {

‎tests/useImplementingTypesAndDefaultNullableToNull/__snapshots__/spec.ts.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const mockD = (overrides?: Partial<D>): D => {
3434
3535
export const mockTest = (overrides?: Partial<Test>): Test => {
3636
return {
37-
field1: overrides && overrides.hasOwnProperty('field1') ? overrides.field1! : mockA() || mockB() || mockC() || mockD(),
37+
field1: overrides && overrides.hasOwnProperty('field1') ? overrides.field1! : (mockA()) || (mockB()) || (mockC()) || (mockD()),
3838
field2: overrides && overrides.hasOwnProperty('field2') ? overrides.field2! : null,
3939
};
4040
};

‎tests/useImplementingTypesAndDefaultNullableToNull/spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ it('should support useImplementingTypes', async () => {
1111
expect(result).toBeDefined();
1212

1313
expect(result).toContain(
14-
"field1: overrides && overrides.hasOwnProperty('field1') ? overrides.field1! : mockA() || mockB() || mockC() || mockD(),",
14+
"field1: overrides && overrides.hasOwnProperty('field1') ? overrides.field1! : (mockA()) || (mockB()) || (mockC()) || (mockD()),",
1515
);
1616

1717
expect(result).toContain("field2: overrides && overrides.hasOwnProperty('field2') ? overrides.field2! : null,");

‎tests/useImplementingTypesAndTerminateCircularRelationships/__snapshots__/spec.ts.snap‎

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,61 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3+
exports[`should parenthesize implementing types with 3+ implementations 1`] = `
4+
"
5+
export const mockQuery = (overrides?: Partial<Query>, _relationshipsToOmit: Set<string> = new Set()): Query => {
6+
const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
7+
relationshipsToOmit.add('Query');
8+
return {
9+
getData: overrides && overrides.hasOwnProperty('getData') ? overrides.getData! : relationshipsToOmit.has('Container') ? {} as Container : mockContainer({}, relationshipsToOmit),
10+
};
11+
};
12+
13+
export const mockContainer = (overrides?: Partial<Container>, _relationshipsToOmit: Set<string> = new Set()): Container => {
14+
const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
15+
relationshipsToOmit.add('Container');
16+
return {
17+
id: overrides && overrides.hasOwnProperty('id') ? overrides.id! : 'vindico',
18+
dataItem: overrides && overrides.hasOwnProperty('dataItem') ? overrides.dataItem! : (relationshipsToOmit.has('TypeA') ? {} as TypeA : mockTypeA({}, relationshipsToOmit)) || (relationshipsToOmit.has('TypeB') ? {} as TypeB : mockTypeB({}, relationshipsToOmit)) || (relationshipsToOmit.has('TypeC') ? {} as TypeC : mockTypeC({}, relationshipsToOmit)),
19+
};
20+
};
21+
22+
export const mockDataItem = (overrides?: Partial<DataItem>, _relationshipsToOmit: Set<string> = new Set()): DataItem => {
23+
const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
24+
relationshipsToOmit.add('DataItem');
25+
return {
26+
name: overrides && overrides.hasOwnProperty('name') ? overrides.name! : 'vitiosus',
27+
};
28+
};
29+
30+
export const mockTypeA = (overrides?: Partial<TypeA>, _relationshipsToOmit: Set<string> = new Set()): TypeA => {
31+
const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
32+
relationshipsToOmit.add('TypeA');
33+
return {
34+
name: overrides && overrides.hasOwnProperty('name') ? overrides.name! : 'demens',
35+
fieldA: overrides && overrides.hasOwnProperty('fieldA') ? overrides.fieldA! : 'occaecati',
36+
};
37+
};
38+
39+
export const mockTypeB = (overrides?: Partial<TypeB>, _relationshipsToOmit: Set<string> = new Set()): TypeB => {
40+
const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
41+
relationshipsToOmit.add('TypeB');
42+
return {
43+
name: overrides && overrides.hasOwnProperty('name') ? overrides.name! : 'vitiosus',
44+
fieldB: overrides && overrides.hasOwnProperty('fieldB') ? overrides.fieldB! : 'deputo',
45+
};
46+
};
47+
48+
export const mockTypeC = (overrides?: Partial<TypeC>, _relationshipsToOmit: Set<string> = new Set()): TypeC => {
49+
const relationshipsToOmit: Set<string> = new Set(_relationshipsToOmit);
50+
relationshipsToOmit.add('TypeC');
51+
return {
52+
name: overrides && overrides.hasOwnProperty('name') ? overrides.name! : 'colo',
53+
fieldC: overrides && overrides.hasOwnProperty('fieldC') ? overrides.fieldC! : 'claustrum',
54+
};
55+
};
56+
"
57+
`;
58+
359
exports[`should support useImplementingTypes and terminateCircularRelationships at the same time 1`] = `
460
"
561
export const mockQuery = (overrides?: Partial<Query>, _relationshipsToOmit: Set<string> = new Set()): Query => {
@@ -15,7 +71,7 @@ export const mockUser = (overrides?: Partial<User>, _relationshipsToOmit: Set<st
1571
relationshipsToOmit.add('User');
1672
return {
1773
id: overrides && overrides.hasOwnProperty('id') ? overrides.id! : 'suscipio',
18-
events: overrides && overrides.hasOwnProperty('events') ? overrides.events! : [relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit) || relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit)],
74+
events: overrides && overrides.hasOwnProperty('events') ? overrides.events! : [(relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit)) || (relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit))],
1975
};
2076
};
2177
@@ -36,7 +92,7 @@ export const mockMeetingEvent = (overrides?: Partial<MeetingEvent>, _relationshi
3692
endDate: overrides && overrides.hasOwnProperty('endDate') ? overrides.endDate! : 'adicio',
3793
startDate: overrides && overrides.hasOwnProperty('startDate') ? overrides.startDate! : 'altus',
3894
timeZone: overrides && overrides.hasOwnProperty('timeZone') ? overrides.timeZone! : 'deprimo',
39-
event: overrides && overrides.hasOwnProperty('event') ? overrides.event! : relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit) || relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit),
95+
event: overrides && overrides.hasOwnProperty('event') ? overrides.event! : (relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit)) || (relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit)),
4096
};
4197
};
4298

‎tests/useImplementingTypesAndTerminateCircularRelationships/spec.ts‎

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { plugin } from '../../src';
22
import testSchema from './schema';
3+
import threeImplSchema from './threeImplSchema';
34

45
it('should support useImplementingTypes and terminateCircularRelationships at the same time', async () => {
56
const result = await plugin(testSchema, [], {
@@ -11,11 +12,28 @@ it('should support useImplementingTypes and terminateCircularRelationships at th
1112
expect(result).toBeDefined();
1213

1314
expect(result).toContain(
14-
"events: overrides && overrides.hasOwnProperty('events') ? overrides.events! : [relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit) || relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit)]",
15+
"events: overrides && overrides.hasOwnProperty('events') ? overrides.events! : [(relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit)) || (relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit))]",
1516
);
1617

1718
expect(result).toContain(
18-
"event: overrides && overrides.hasOwnProperty('event') ? overrides.event! : relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit) || relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit)",
19+
"event: overrides && overrides.hasOwnProperty('event') ? overrides.event! : (relationshipsToOmit.has('MeetingEvent') ? {} as MeetingEvent : mockMeetingEvent({}, relationshipsToOmit)) || (relationshipsToOmit.has('OtherEvent') ? {} as OtherEvent : mockOtherEvent({}, relationshipsToOmit))",
20+
);
21+
22+
expect(result).toMatchSnapshot();
23+
});
24+
25+
it('should parenthesize implementing types with 3+ implementations', async () => {
26+
const result = await plugin(threeImplSchema, [], {
27+
prefix: 'mock',
28+
useImplementingTypes: true,
29+
terminateCircularRelationships: true,
30+
});
31+
32+
expect(result).toBeDefined();
33+
34+
// Each ternary in the || chain must be wrapped in parentheses
35+
expect(result).toContain(
36+
"dataItem: overrides && overrides.hasOwnProperty('dataItem') ? overrides.dataItem! : (relationshipsToOmit.has('TypeA') ? {} as TypeA : mockTypeA({}, relationshipsToOmit)) || (relationshipsToOmit.has('TypeB') ? {} as TypeB : mockTypeB({}, relationshipsToOmit)) || (relationshipsToOmit.has('TypeC') ? {} as TypeC : mockTypeC({}, relationshipsToOmit))",
1937
);
2038

2139
expect(result).toMatchSnapshot();

‎tests/useImplementingTypesAndTerminateCircularRelationships/threeImplSchema.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { buildSchema } from 'graphql';
2+
3+
export default buildSchema(/* GraphQL */ `
4+
type Query {
5+
getData(id: String!): Container
6+
}
7+
8+
type Container {
9+
id: String!
10+
dataItem: DataItem!
11+
}
12+
13+
interface DataItem {
14+
name: String
15+
}
16+
17+
type TypeA implements DataItem {
18+
name: String
19+
fieldA: String
20+
}
21+
22+
type TypeB implements DataItem {
23+
name: String
24+
fieldB: String
25+
}
26+
27+
type TypeC implements DataItem {
28+
name: String
29+
fieldC: String
30+
}
31+
`);

0 commit comments

Comments
 (0)