Skip to content

Commit 6245082

Browse files
jdollejdolle
committed
Finish adding criticality override on new type changes
1 parent c13c1c7 commit 6245082

File tree

13 files changed

+131
-40
lines changed

13 files changed

+131
-40
lines changed

‎packages/core/src/diff/argument.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,16 @@ export function changesInArgument(
4444
compareLists(oldArg?.astNode?.directives || [], newArg.astNode.directives || [], {
4545
onAdded(directive) {
4646
addChange(
47-
directiveUsageAdded(Kind.ARGUMENT, directive, {
48-
argument: newArg,
49-
field,
50-
type,
51-
}),
47+
directiveUsageAdded(
48+
Kind.ARGUMENT,
49+
directive,
50+
{
51+
argument: newArg,
52+
field,
53+
type,
54+
},
55+
oldArg === null,
56+
),
5257
);
5358
},
5459

‎packages/core/src/diff/changes/change.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ export type DirectiveUsageUnionMemberAddedChange = {
405405
unionName: string;
406406
addedUnionMemberTypeName: string;
407407
addedDirectiveName: string;
408+
addedToNewType: boolean;
408409
};
409410
};
410411

@@ -516,6 +517,7 @@ export type ObjectTypeInterfaceAddedChange = {
516517
meta: {
517518
objectTypeName: string;
518519
addedInterfaceName: string;
520+
isSafeChange: boolean;
519521
};
520522
};
521523

@@ -633,6 +635,7 @@ export type UnionMemberAddedChange = {
633635
meta: {
634636
unionName: string;
635637
addedUnionMemberTypeName: string;
638+
isSafeChange: boolean;
636639
};
637640
};
638641

@@ -643,6 +646,7 @@ export type DirectiveUsageEnumAddedChange = {
643646
meta: {
644647
enumName: string;
645648
addedDirectiveName: string;
649+
addedToNewType: boolean;
646650
};
647651
};
648652

@@ -660,6 +664,7 @@ export type DirectiveUsageEnumValueAddedChange = {
660664
enumName: string;
661665
enumValueName: string;
662666
addedDirectiveName: string;
667+
addedToNewType: boolean;
663668
};
664669
};
665670

@@ -691,6 +696,7 @@ export type DirectiveUsageInputObjectAddedChange = {
691696
isAddedInputFieldTypeNullable: boolean;
692697
addedInputFieldType: string;
693698
addedDirectiveName: string;
699+
addedToNewType: boolean;
694700
};
695701
};
696702

@@ -700,6 +706,7 @@ export type DirectiveUsageInputFieldDefinitionAddedChange = {
700706
inputObjectName: string;
701707
inputFieldName: string;
702708
addedDirectiveName: string;
709+
addedToNewType: boolean;
703710
};
704711
};
705712

@@ -735,6 +742,7 @@ export type DirectiveUsageScalarAddedChange = {
735742
meta: {
736743
scalarName: string;
737744
addedDirectiveName: string;
745+
addedToNewType: boolean;
738746
};
739747
};
740748

@@ -751,6 +759,7 @@ export type DirectiveUsageObjectAddedChange = {
751759
meta: {
752760
objectName: string;
753761
addedDirectiveName: string;
762+
addedToNewType: boolean;
754763
};
755764
};
756765

@@ -767,6 +776,7 @@ export type DirectiveUsageInterfaceAddedChange = {
767776
meta: {
768777
interfaceName: string;
769778
addedDirectiveName: string;
779+
addedToNewType: boolean;
770780
};
771781
};
772782

@@ -775,6 +785,7 @@ export type DirectiveUsageSchemaAddedChange = {
775785
meta: {
776786
addedDirectiveName: string;
777787
schemaTypeName: string;
788+
addedToNewType: boolean;
778789
};
779790
};
780791

@@ -792,6 +803,7 @@ export type DirectiveUsageFieldDefinitionAddedChange = {
792803
typeName: string;
793804
fieldName: string;
794805
addedDirectiveName: string;
806+
addedToNewType: boolean;
795807
};
796808
};
797809

@@ -811,6 +823,7 @@ export type DirectiveUsageArgumentDefinitionChange = {
811823
fieldName: string;
812824
argumentName: string;
813825
addedDirectiveName: string;
826+
addedToNewType: boolean;
814827
};
815828
};
816829

‎packages/core/src/diff/changes/directive-usage.ts‎

Lines changed: 45 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ export function directiveUsageArgumentDefinitionAddedFromMeta(
138138
) {
139139
return {
140140
criticality: {
141-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
141+
level: args.meta.addedToNewType
142+
? CriticalityLevel.NonBreaking
143+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
142144
reason: `Directive '${args.meta.addedDirectiveName}' was added to argument '${args.meta.argumentName}'`,
143145
},
144146
type: ChangeType.DirectiveUsageArgumentDefinitionAdded,
@@ -188,7 +190,9 @@ function buildDirectiveUsageInputObjectAddedMessage(
188190
export function directiveUsageInputObjectAddedFromMeta(args: DirectiveUsageInputObjectAddedChange) {
189191
return {
190192
criticality: {
191-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
193+
level: args.meta.addedToNewType
194+
? CriticalityLevel.NonBreaking
195+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
192196
reason: `Directive '${args.meta.addedDirectiveName}' was added to input object '${args.meta.inputObjectName}'`,
193197
},
194198
type: ChangeType.DirectiveUsageInputObjectAdded,
@@ -228,7 +232,9 @@ function buildDirectiveUsageInterfaceAddedMessage(
228232
export function directiveUsageInterfaceAddedFromMeta(args: DirectiveUsageInterfaceAddedChange) {
229233
return {
230234
criticality: {
231-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
235+
level: args.meta.addedToNewType
236+
? CriticalityLevel.NonBreaking
237+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
232238
reason: `Directive '${args.meta.addedDirectiveName}' was added to interface '${args.meta.interfaceName}'`,
233239
},
234240
type: ChangeType.DirectiveUsageInterfaceAdded,
@@ -268,7 +274,9 @@ export function directiveUsageInputFieldDefinitionAddedFromMeta(
268274
) {
269275
return {
270276
criticality: {
271-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
277+
level: args.meta.addedToNewType
278+
? CriticalityLevel.NonBreaking
279+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
272280
reason: `Directive '${args.meta.addedDirectiveName}' was added to input field '${args.meta.inputFieldName}'`,
273281
},
274282
type: ChangeType.DirectiveUsageInputFieldDefinitionAdded,
@@ -314,7 +322,9 @@ function buildDirectiveUsageObjectAddedMessage(
314322
export function directiveUsageObjectAddedFromMeta(args: DirectiveUsageObjectAddedChange) {
315323
return {
316324
criticality: {
317-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
325+
level: args.meta.addedToNewType
326+
? CriticalityLevel.NonBreaking
327+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
318328
reason: `Directive '${args.meta.addedDirectiveName}' was added to object '${args.meta.objectName}'`,
319329
},
320330
type: ChangeType.DirectiveUsageObjectAdded,
@@ -350,7 +360,9 @@ function buildDirectiveUsageEnumAddedMessage(args: DirectiveUsageEnumAddedChange
350360
export function directiveUsageEnumAddedFromMeta(args: DirectiveUsageEnumAddedChange) {
351361
return {
352362
criticality: {
353-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
363+
level: args.meta.addedToNewType
364+
? CriticalityLevel.NonBreaking
365+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
354366
reason: `Directive '${args.meta.addedDirectiveName}' was added to enum '${args.meta.enumName}'`,
355367
},
356368
type: ChangeType.DirectiveUsageEnumAdded,
@@ -390,7 +402,9 @@ export function directiveUsageFieldDefinitionAddedFromMeta(
390402
) {
391403
return {
392404
criticality: {
393-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
405+
level: args.meta.addedToNewType
406+
? CriticalityLevel.NonBreaking
407+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
394408
reason: `Directive '${args.meta.addedDirectiveName}' was added to field '${args.meta.fieldName}'`,
395409
},
396410
type: ChangeType.DirectiveUsageFieldDefinitionAdded,
@@ -430,7 +444,9 @@ function buildDirectiveUsageEnumValueAddedMessage(
430444
export function directiveUsageEnumValueAddedFromMeta(args: DirectiveUsageEnumValueAddedChange) {
431445
return {
432446
criticality: {
433-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
447+
level: args.meta.addedToNewType
448+
? CriticalityLevel.NonBreaking
449+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
434450
reason: `Directive '${args.meta.addedDirectiveName}' was added to enum value '${args.meta.enumName}.${args.meta.enumValueName}'`,
435451
},
436452
type: ChangeType.DirectiveUsageEnumValueAdded,
@@ -468,7 +484,9 @@ function buildDirectiveUsageSchemaAddedMessage(
468484
export function directiveUsageSchemaAddedFromMeta(args: DirectiveUsageSchemaAddedChange) {
469485
return {
470486
criticality: {
471-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
487+
level: args.meta.addedToNewType
488+
? CriticalityLevel.NonBreaking
489+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
472490
reason: `Directive '${args.meta.addedDirectiveName}' was added to schema '${args.meta.schemaTypeName}'`,
473491
},
474492
type: ChangeType.DirectiveUsageSchemaAdded,
@@ -506,7 +524,9 @@ function buildDirectiveUsageScalarAddedMessage(
506524
export function directiveUsageScalarAddedFromMeta(args: DirectiveUsageScalarAddedChange) {
507525
return {
508526
criticality: {
509-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
527+
level: args.meta.addedToNewType
528+
? CriticalityLevel.NonBreaking
529+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
510530
reason: `Directive '${args.meta.addedDirectiveName}' was added to scalar '${args.meta.scalarName}'`,
511531
},
512532
type: ChangeType.DirectiveUsageScalarAdded,
@@ -544,7 +564,9 @@ function buildDirectiveUsageUnionMemberAddedMessage(
544564
export function directiveUsageUnionMemberAddedFromMeta(args: DirectiveUsageUnionMemberAddedChange) {
545565
return {
546566
criticality: {
547-
level: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
567+
level: args.meta.addedToNewType
568+
? CriticalityLevel.NonBreaking
569+
: addedSpecialDirective(args.meta.addedDirectiveName, CriticalityLevel.Dangerous),
548570
reason: `Directive '${args.meta.addedDirectiveName}' was added to union member '${args.meta.unionName}.${args.meta.addedUnionMemberTypeName}'`,
549571
},
550572
type: ChangeType.DirectiveUsageUnionMemberAdded,
@@ -579,6 +601,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
579601
kind: K,
580602
directive: ConstDirectiveNode,
581603
payload: KindToPayload[K]['input'],
604+
addedToNewType: boolean,
582605
): Change {
583606
if (isOfKind(kind, Kind.ARGUMENT, payload)) {
584607
return directiveUsageArgumentDefinitionAddedFromMeta({
@@ -588,6 +611,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
588611
argumentName: payload.argument.name,
589612
fieldName: payload.field.name,
590613
typeName: payload.type.name,
614+
addedToNewType,
591615
},
592616
});
593617
}
@@ -598,6 +622,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
598622
addedDirectiveName: directive.name.value,
599623
inputFieldName: payload.field.name,
600624
inputObjectName: payload.type.name,
625+
addedToNewType,
601626
},
602627
});
603628
}
@@ -610,6 +635,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
610635
addedInputFieldType: payload.name,
611636
inputObjectName: payload.name,
612637
isAddedInputFieldTypeNullable: kind === Kind.INPUT_VALUE_DEFINITION,
638+
addedToNewType,
613639
},
614640
});
615641
}
@@ -619,6 +645,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
619645
meta: {
620646
addedDirectiveName: directive.name.value,
621647
interfaceName: payload.name,
648+
addedToNewType,
622649
},
623650
});
624651
}
@@ -628,6 +655,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
628655
meta: {
629656
objectName: payload.name,
630657
addedDirectiveName: directive.name.value,
658+
addedToNewType,
631659
},
632660
});
633661
}
@@ -637,6 +665,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
637665
meta: {
638666
enumName: payload.name,
639667
addedDirectiveName: directive.name.value,
668+
addedToNewType,
640669
},
641670
});
642671
}
@@ -647,6 +676,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
647676
addedDirectiveName: directive.name.value,
648677
fieldName: payload.field.name,
649678
typeName: payload.parentType.name,
679+
addedToNewType,
650680
},
651681
});
652682
}
@@ -657,6 +687,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
657687
addedDirectiveName: directive.name.value,
658688
addedUnionMemberTypeName: payload.name,
659689
unionName: payload.name,
690+
addedToNewType,
660691
},
661692
});
662693
}
@@ -667,6 +698,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
667698
enumName: payload.type.name,
668699
enumValueName: payload.value.name,
669700
addedDirectiveName: directive.name.value,
701+
addedToNewType,
670702
},
671703
});
672704
}
@@ -676,6 +708,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
676708
meta: {
677709
addedDirectiveName: directive.name.value,
678710
schemaTypeName: payload.getQueryType()?.name || '',
711+
addedToNewType,
679712
},
680713
});
681714
}
@@ -685,6 +718,7 @@ export function directiveUsageAdded<K extends keyof KindToPayload>(
685718
meta: {
686719
scalarName: payload.name,
687720
addedDirectiveName: directive.name.value,
721+
addedToNewType,
688722
},
689723
});
690724
}

‎packages/core/src/diff/changes/object.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export function objectTypeInterfaceAddedFromMeta(args: ObjectTypeInterfaceAddedC
1515
return {
1616
type: ChangeType.ObjectTypeInterfaceAdded,
1717
criticality: {
18-
level: CriticalityLevel.Dangerous,
18+
level: args.meta.isSafeChange ? CriticalityLevel.NonBreaking : CriticalityLevel.Dangerous,
1919
reason:
2020
'Adding an interface to an object type may break existing clients that were not programming defensively against a new possible type.',
2121
},
@@ -28,12 +28,14 @@ export function objectTypeInterfaceAddedFromMeta(args: ObjectTypeInterfaceAddedC
2828
export function objectTypeInterfaceAdded(
2929
iface: GraphQLInterfaceType,
3030
type: GraphQLObjectType | GraphQLInterfaceType,
31+
isSafeChange: boolean,
3132
): Change<typeof ChangeType.ObjectTypeInterfaceAdded> {
3233
return objectTypeInterfaceAddedFromMeta({
3334
type: ChangeType.ObjectTypeInterfaceAdded,
3435
meta: {
3536
objectTypeName: type.name,
3637
addedInterfaceName: iface.name,
38+
isSafeChange,
3739
},
3840
});
3941
}

‎packages/core/src/diff/changes/union.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ function buildUnionMemberAddedMessage(args: UnionMemberAddedChange['meta']) {
4545
export function buildUnionMemberAddedMessageFromMeta(args: UnionMemberAddedChange) {
4646
return {
4747
criticality: {
48-
level: CriticalityLevel.Dangerous,
48+
level: args.meta.isSafeChange ? CriticalityLevel.NonBreaking : CriticalityLevel.Dangerous,
4949
reason:
5050
'Adding a possible type to Unions may break existing clients that were not programming defensively against a new possible type.',
5151
},
@@ -59,12 +59,14 @@ export function buildUnionMemberAddedMessageFromMeta(args: UnionMemberAddedChang
5959
export function unionMemberAdded(
6060
union: GraphQLUnionType,
6161
type: GraphQLObjectType,
62+
isSafeChange: boolean,
6263
): Change<typeof ChangeType.UnionMemberAdded> {
6364
return buildUnionMemberAddedMessageFromMeta({
6465
type: ChangeType.UnionMemberAdded,
6566
meta: {
6667
unionName: union.name,
6768
addedUnionMemberTypeName: type.name,
69+
isSafeChange,
6870
},
6971
});
7072
}

0 commit comments

Comments
 (0)