Skip to content

Commit 2056307

Browse files
jdollen1ru4l
jdolle
and
n1ru4l
authored
feat: Add setting to make dangerous changes into breaking (#6626)
Co-authored-by: Laurin Quast <[email protected]>
1 parent df3e5a2 commit 2056307

File tree

21 files changed

+559
-244
lines changed

21 files changed

+559
-244
lines changed

‎.changeset/chatty-masks-love.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'hive': minor
3+
---
4+
5+
Add target breaking change setting to turn dangerous changes into breaking changes

‎.changeset/pretty-schools-confess.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-hive/cli': minor
3+
---
4+
5+
Show dangerous changes as a separate list in schema:check

‎integration-tests/tests/cli/__snapshots__/schema.spec.ts.snap

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ stdout--------------------------------------------:
1515
1616
Breaking changes:
1717
- Field email was removed from object type User
18-
Safe changes:
18+
Dangerous changes:
1919
- Enum value VIEWER was added to enum UserRole
20+
Safe changes:
2021
- Field address was added to object type User
2122
- Field User.role changed type from UserRole to UserRole!
2223
@@ -275,8 +276,9 @@ stdout--------------------------------------------:
275276
276277
Breaking changes:
277278
- Field email was removed from object type User
278-
Safe changes:
279+
Dangerous changes:
279280
- Enum value VIEWER was added to enum UserRole
281+
Safe changes:
280282
- Field address was added to object type User
281283
- Field User.role changed type from UserRole to UserRole!
282284
@@ -524,8 +526,9 @@ stdout--------------------------------------------:
524526
525527
Breaking changes:
526528
- Field email was removed from object type User
527-
Safe changes:
529+
Dangerous changes:
528530
- Enum value VIEWER was added to enum UserRole
531+
Safe changes:
529532
- Field address was added to object type User
530533
- Field User.role changed type from UserRole to UserRole!
531534

‎packages/libraries/cli/src/helpers/schema.ts

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,37 @@ export const renderChanges = (maskedChanges: FragmentType<typeof RenderChanges_S
7373
const breakingChanges = changes.nodes.filter(
7474
change => change.criticality === CriticalityLevel.Breaking,
7575
);
76-
const safeChanges = changes.nodes.filter(
77-
change => change.criticality !== CriticalityLevel.Breaking,
76+
const dangerousChanges = changes.nodes.filter(
77+
change => change.criticality === CriticalityLevel.Dangerous,
78+
);
79+
const safeChanges = changes.nodes.filter(change => change.criticality === CriticalityLevel.Safe);
80+
81+
const otherChanges = changes.nodes.filter(
82+
change => !Object.keys(CriticalityLevel).includes(change.criticality),
7883
);
7984

8085
if (breakingChanges.length) {
8186
t.indent(`Breaking changes:`);
8287
writeChanges(breakingChanges);
8388
}
8489

90+
if (dangerousChanges.length) {
91+
t.indent(`Dangerous changes:`);
92+
writeChanges(dangerousChanges);
93+
}
94+
8595
if (safeChanges.length) {
8696
t.indent(`Safe changes:`);
8797
writeChanges(safeChanges);
8898
}
8999

100+
// For backwards compatibility in case more criticality levels are added.
101+
// This is unlikely to happen.
102+
if (otherChanges.length) {
103+
t.indent(`Other changes: (Current CLI version does not support these CriticalityLevels)`);
104+
writeChanges(otherChanges);
105+
}
106+
90107
return t.state.value;
91108
};
92109

‎packages/migrations/src/actions/2025.03.20T00-00-00.dangerous_breaking.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import type { MigrationExecutor } from '../pg-migrator';
2+
3+
export default {
4+
name: '2025.03.20T00-00-00.dangerous_breaking.ts',
5+
noTransaction: true,
6+
run: ({ sql }) => sql`
7+
ALTER TABLE
8+
targets
9+
ADD COLUMN
10+
fail_diff_on_dangerous_change BOOLEAN NOT NULL DEFAULT FALSE;
11+
`,
12+
} satisfies MigrationExecutor;

‎packages/migrations/src/run-pg-migrations.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,5 +161,6 @@ export const runPGMigrations = async (args: { slonik: DatabasePool; runTo?: stri
161161
await import('./actions/2025.02.20T00-00-00.organization-access-tokens'),
162162
await import('./actions/2025.02.14T00-00-00.schema-versions-metadata'),
163163
await import('./actions/2025.02.21T00-00-00.schema-versions-metadata-attributes'),
164+
await import('./actions/2025.03.20T00-00-00.dangerous_breaking'),
164165
],
165166
});

‎packages/services/api/src/modules/schema/providers/models/composite.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export class CompositeModel {
5353
> | null;
5454
compositionCheck: Awaited<ReturnType<RegistryChecks['composition']>>;
5555
conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
56+
failDiffOnDangerousChange: null | boolean;
5657
}): Promise<Array<ContractCheckInput> | null> {
5758
const contractResults = (args.compositionCheck.result ?? args.compositionCheck.reason)
5859
?.contracts;
@@ -80,6 +81,7 @@ export class CompositeModel {
8081
approvedChanges: contract.approvedChanges ?? null,
8182
existingSdl: contract.latestValidVersion?.compositeSchemaSdl ?? null,
8283
incomingSdl: contractCompositionResult?.result?.fullSchemaSdl ?? null,
84+
failDiffOnDangerousChange: args.failDiffOnDangerousChange,
8385
}),
8486
};
8587
}),
@@ -104,6 +106,7 @@ export class CompositeModel {
104106
approvedChanges,
105107
conditionalBreakingChangeDiffConfig,
106108
contracts,
109+
failDiffOnDangerousChange,
107110
}: {
108111
input: {
109112
sdl: string;
@@ -131,6 +134,7 @@ export class CompositeModel {
131134
organization: Organization;
132135
approvedChanges: Map<string, SchemaChangeType>;
133136
conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
137+
failDiffOnDangerousChange: null | boolean;
134138
contracts: Array<
135139
ContractInput & {
136140
approvedChanges: Map<string, SchemaChangeType> | null;
@@ -222,6 +226,7 @@ export class CompositeModel {
222226
contracts,
223227
compositionCheck,
224228
conditionalBreakingChangeDiffConfig,
229+
failDiffOnDangerousChange,
225230
});
226231
this.logger.info('Contract checks: %o', contractChecks);
227232

@@ -234,6 +239,7 @@ export class CompositeModel {
234239
incomingSdl:
235240
compositionCheck.result?.fullSchemaSdl ?? compositionCheck.reason?.fullSchemaSdl ?? null,
236241
conditionalBreakingChangeConfig: conditionalBreakingChangeDiffConfig,
242+
failDiffOnDangerousChange,
237243
}),
238244
this.checks.policyCheck({
239245
selector,
@@ -304,6 +310,7 @@ export class CompositeModel {
304310
baseSchema,
305311
contracts,
306312
conditionalBreakingChangeDiffConfig,
313+
failDiffOnDangerousChange,
307314
}: {
308315
input: PublishInput;
309316
project: Project;
@@ -323,6 +330,7 @@ export class CompositeModel {
323330
baseSchema: string | null;
324331
contracts: Array<ContractInput> | null;
325332
conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
333+
failDiffOnDangerousChange: null | boolean;
326334
}): Promise<SchemaPublishResult> {
327335
const incoming: PushedCompositeSchema = {
328336
kind: 'composite',
@@ -478,12 +486,14 @@ export class CompositeModel {
478486
approvedChanges: null,
479487
existingSdl: previousVersionSdl,
480488
incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
489+
failDiffOnDangerousChange,
481490
});
482491

483492
const contractChecks = await this.getContractChecks({
484493
contracts,
485494
compositionCheck,
486495
conditionalBreakingChangeDiffConfig,
496+
failDiffOnDangerousChange,
487497
});
488498

489499
const messages: string[] = [];
@@ -545,6 +555,7 @@ export class CompositeModel {
545555
baseSchema,
546556
conditionalBreakingChangeDiffConfig,
547557
contracts,
558+
failDiffOnDangerousChange,
548559
}: {
549560
input: {
550561
serviceName: string;
@@ -569,6 +580,7 @@ export class CompositeModel {
569580
} | null;
570581
contracts: Array<ContractInput> | null;
571582
conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
583+
failDiffOnDangerousChange: null | boolean;
572584
}): Promise<SchemaDeleteResult> {
573585
const incoming: DeletedCompositeSchema = {
574586
kind: 'composite',
@@ -645,12 +657,14 @@ export class CompositeModel {
645657
approvedChanges: null,
646658
existingSdl: previousVersionSdl,
647659
incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
660+
failDiffOnDangerousChange,
648661
});
649662

650663
const contractChecks = await this.getContractChecks({
651664
contracts,
652665
compositionCheck,
653666
conditionalBreakingChangeDiffConfig,
667+
failDiffOnDangerousChange,
654668
});
655669

656670
if (

‎packages/services/api/src/modules/schema/providers/models/single.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export class SingleModel {
4444
baseSchema,
4545
approvedChanges,
4646
conditionalBreakingChangeDiffConfig,
47+
failDiffOnDangerousChange,
4748
}: {
4849
input: {
4950
sdl: string;
@@ -68,6 +69,7 @@ export class SingleModel {
6869
organization: Organization;
6970
approvedChanges: Map<string, SchemaChangeType>;
7071
conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
72+
failDiffOnDangerousChange: boolean;
7173
}): Promise<SchemaCheckResult> {
7274
const incoming: SingleSchema = {
7375
kind: 'single',
@@ -131,6 +133,7 @@ export class SingleModel {
131133
approvedChanges,
132134
existingSdl: previousVersionSdl,
133135
incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
136+
failDiffOnDangerousChange,
134137
}),
135138
this.checks.policyCheck({
136139
selector,
@@ -178,6 +181,7 @@ export class SingleModel {
178181
latestComposable,
179182
baseSchema,
180183
conditionalBreakingChangeDiffConfig,
184+
failDiffOnDangerousChange,
181185
}: {
182186
input: PublishInput;
183187
organization: Organization;
@@ -195,6 +199,7 @@ export class SingleModel {
195199
} | null;
196200
baseSchema: string | null;
197201
conditionalBreakingChangeDiffConfig: null | ConditionalBreakingChangeDiffConfig;
202+
failDiffOnDangerousChange: boolean;
198203
}): Promise<SchemaPublishResult> {
199204
const incoming: SingleSchema = {
200205
kind: 'single',
@@ -267,6 +272,7 @@ export class SingleModel {
267272
approvedChanges: null,
268273
existingSdl: previousVersionSdl,
269274
incomingSdl: compositionCheck.result?.fullSchemaSdl ?? null,
275+
failDiffOnDangerousChange,
270276
}),
271277
]);
272278

‎packages/services/api/src/modules/schema/providers/registry-checks.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,7 @@ export class RegistryChecks {
425425
approvedChanges: null | Map<string, SchemaChangeType>;
426426
/** Settings for fetching conditional breaking changes. */
427427
conditionalBreakingChangeConfig: null | ConditionalBreakingChangeDiffConfig;
428+
failDiffOnDangerousChange: null | boolean;
428429
}) {
429430
let existingSchema: GraphQLSchema | null = null;
430431
let incomingSchema: GraphQLSchema | null = null;
@@ -542,7 +543,10 @@ export class RegistryChecks {
542543
const coordinatesDiff = diffSchemaCoordinates(existingSchema, incomingSchema);
543544

544545
for (const change of inspectorChanges) {
545-
if (change.criticality === CriticalityLevel.Breaking) {
546+
if (
547+
change.criticality === CriticalityLevel.Breaking ||
548+
(args.failDiffOnDangerousChange && change.criticality === CriticalityLevel.Dangerous)
549+
) {
546550
if (change.isSafeBasedOnUsage === true) {
547551
breakingChanges.push(change);
548552
continue;

0 commit comments

Comments
 (0)