Skip to content

Commit f61c716

Browse files
jdollejdolle
committed
Change path for deprecated directives; fix duplicating deprecated directives on path
1 parent 563b054 commit f61c716

File tree

8 files changed

+108
-59
lines changed

8 files changed

+108
-59
lines changed

‎packages/core/__tests__/diff/interface.test.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,9 @@ describe('interface', () => {
168168

169169
const changes = await diff(a, b);
170170
const change = {
171-
a: findFirstChangeByPath(changes, 'Foo.a'),
172-
b: findFirstChangeByPath(changes, 'Foo.b'),
173-
c: findFirstChangeByPath(changes, 'Foo.c'),
171+
a: findFirstChangeByPath(changes, 'Foo.a.@deprecated'),
172+
b: findFirstChangeByPath(changes, 'Foo.b.@deprecated'),
173+
c: findFirstChangeByPath(changes, 'Foo.c.@deprecated'),
174174
};
175175

176176
// Changed
@@ -205,8 +205,8 @@ describe('interface', () => {
205205

206206
const changes = await diff(a, b);
207207
const change = {
208-
a: findFirstChangeByPath(changes, 'Foo.a'),
209-
b: findFirstChangeByPath(changes, 'Foo.b'),
208+
a: findFirstChangeByPath(changes, 'Foo.a.@deprecated'),
209+
b: findFirstChangeByPath(changes, 'Foo.b.@deprecated'),
210210
};
211211

212212
// Changed
@@ -234,8 +234,9 @@ describe('interface', () => {
234234

235235
const changes = await diff(a, b);
236236

237-
expect(findChangesByPath(changes, 'Foo.a')).toHaveLength(1);
238-
const change = findFirstChangeByPath(changes, 'Foo.a');
237+
// one for deprecation added, and one for the reason added
238+
expect(findChangesByPath(changes, 'Foo.a.@deprecated')).toHaveLength(2);
239+
const change = findFirstChangeByPath(changes, 'Foo.a.@deprecated');
239240

240241
// added
241242
expect(change.criticality.level).toEqual(CriticalityLevel.NonBreaking);

‎packages/core/__tests__/diff/object.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,9 @@ describe('object', () => {
329329

330330
const changes = await diff(a, b);
331331
const change = {
332-
a: findFirstChangeByPath(changes, 'Foo.a'),
333-
b: findFirstChangeByPath(changes, 'Foo.b'),
334-
c: findFirstChangeByPath(changes, 'Foo.c'),
332+
a: findFirstChangeByPath(changes, 'Foo.a.@deprecated'),
333+
b: findFirstChangeByPath(changes, 'Foo.b.@deprecated'),
334+
c: findFirstChangeByPath(changes, 'Foo.c.@deprecated'),
335335
};
336336

337337
// Changed
@@ -366,8 +366,8 @@ describe('object', () => {
366366

367367
const changes = await diff(a, b);
368368
const change = {
369-
a: findFirstChangeByPath(changes, 'Foo.a'),
370-
b: findFirstChangeByPath(changes, 'Foo.b'),
369+
a: findFirstChangeByPath(changes, 'Foo.a.@deprecated'),
370+
b: findFirstChangeByPath(changes, 'Foo.b.@deprecated'),
371371
};
372372

373373
// Changed

‎packages/core/__tests__/diff/schema.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ test('huge test', async () => {
359359
'CType',
360360
'CType.b',
361361
'CType.c',
362-
'CType.a',
362+
'CType.a.@deprecated',
363363
'CType.a.arg',
364364
'CType.d.arg',
365365
'MyUnion',

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ export function fieldDeprecationAddedFromMeta(args: FieldDeprecationAddedChange)
199199
},
200200
message: buildFieldDeprecatedAddedMessage(args.meta),
201201
meta: args.meta,
202-
path: [args.meta.typeName, args.meta.fieldName].join('.'),
202+
path: [args.meta.typeName, args.meta.fieldName, `@${GraphQLDeprecatedDirective.name}`].join(
203+
'.',
204+
),
203205
} as const;
204206
}
205207

@@ -226,7 +228,9 @@ export function fieldDeprecationRemovedFromMeta(args: FieldDeprecationRemovedCha
226228
},
227229
message: `Field '${args.meta.typeName}.${args.meta.fieldName}' is no longer deprecated`,
228230
meta: args.meta,
229-
path: [args.meta.typeName, args.meta.fieldName].join('.'),
231+
path: [args.meta.typeName, args.meta.fieldName, `@${GraphQLDeprecatedDirective.name}`].join(
232+
'.',
233+
),
230234
} as const;
231235
}
232236

@@ -257,7 +261,9 @@ export function fieldDeprecationReasonChangedFromMeta(args: FieldDeprecationReas
257261
},
258262
message: buildFieldDeprecationReasonChangedMessage(args.meta),
259263
meta: args.meta,
260-
path: [args.meta.typeName, args.meta.fieldName].join('.'),
264+
path: [args.meta.typeName, args.meta.fieldName, `@${GraphQLDeprecatedDirective.name}`].join(
265+
'.',
266+
),
261267
} as const;
262268
}
263269

‎packages/patch/src/__tests__/directive-usage.test.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,22 @@ const baseSchema = /* GraphQL */ `
4343
`;
4444

4545
describe('directiveUsages: added', () => {
46+
test('directiveUsageFieldDefinitionAdded: @deprecated', async () => {
47+
const before = `
48+
type Foo {
49+
new: String
50+
old: String
51+
}
52+
`;
53+
const after = /* GraphQL */ `
54+
type Foo {
55+
new: String
56+
old: String @deprecated(reason: "No good")
57+
}
58+
`;
59+
await expectPatchToMatch(before, after);
60+
});
61+
4662
test('directiveUsageArgumentDefinitionAdded', async () => {
4763
const before = baseSchema;
4864
const after = /* GraphQL */ `

‎packages/patch/src/patches/directive-usages.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ function directiveUsageDefinitionAdded(
6161

6262
const directiveNode = nodeByPath.get(change.path);
6363
const parentNode = nodeByPath.get(parentPath(change.path)) as
64-
| { directives?: DirectiveNode[] }
64+
| { kind: Kind; directives?: DirectiveNode[] }
6565
| undefined;
6666
if (directiveNode) {
6767
handleError(
@@ -70,6 +70,13 @@ function directiveUsageDefinitionAdded(
7070
config,
7171
);
7272
} else if (parentNode) {
73+
if (
74+
change.meta.addedDirectiveName === 'deprecated' &&
75+
(parentNode.kind === Kind.FIELD_DEFINITION || parentNode.kind === Kind.ENUM_VALUE_DEFINITION)
76+
) {
77+
return; // ignore because deprecated is handled by its own change... consider adjusting this.
78+
}
79+
7380
const newDirective: DirectiveNode = {
7481
kind: Kind.DIRECTIVE,
7582
name: nameNode(change.meta.addedDirectiveName),

‎packages/patch/src/patches/fields.ts‎

Lines changed: 60 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
findNamedNode,
3737
getChangedNodeOfKind,
3838
getDeletedNodeOfKind,
39+
getDeletedParentNodeOfKind,
3940
getDeprecatedDirectiveNode,
4041
parentPath,
4142
} from '../utils.js';
@@ -370,38 +371,48 @@ export function fieldDeprecationAdded(
370371
nodeByPath: Map<string, ASTNode>,
371372
config: PatchConfig,
372373
) {
373-
const fieldNode = getChangedNodeOfKind(change, nodeByPath, Kind.FIELD_DEFINITION, config);
374-
if (fieldNode) {
375-
const hasExistingDeprecationDirective = getDeprecatedDirectiveNode(fieldNode);
376-
if (hasExistingDeprecationDirective) {
377-
handleError(
378-
change,
379-
new AddedCoordinateAlreadyExistsError(Kind.DIRECTIVE, '@deprecated'),
380-
config,
381-
);
382-
} else {
383-
const directiveNode = {
384-
kind: Kind.DIRECTIVE,
385-
name: nameNode(GraphQLDeprecatedDirective.name),
386-
...(change.meta.deprecationReason &&
387-
change.meta.deprecationReason !== DEPRECATION_REASON_DEFAULT
388-
? {
389-
arguments: [
390-
{
391-
kind: Kind.ARGUMENT,
392-
name: nameNode('reason'),
393-
value: stringNode(change.meta.deprecationReason),
394-
},
395-
],
396-
}
397-
: {}),
398-
} as DirectiveNode;
374+
if (assertChangeHasPath(change, config)) {
375+
const fieldNode = nodeByPath.get(parentPath(change.path));
376+
if (fieldNode) {
377+
if (fieldNode.kind !== Kind.FIELD_DEFINITION) {
378+
handleError(
379+
change,
380+
new ChangedCoordinateKindMismatchError(Kind.FIELD_DEFINITION, fieldNode.kind),
381+
config,
382+
);
383+
return;
384+
}
385+
const hasExistingDeprecationDirective = getDeprecatedDirectiveNode(fieldNode);
386+
if (hasExistingDeprecationDirective) {
387+
handleError(
388+
change,
389+
new AddedCoordinateAlreadyExistsError(Kind.DIRECTIVE, '@deprecated'),
390+
config,
391+
);
392+
} else {
393+
const directiveNode = {
394+
kind: Kind.DIRECTIVE,
395+
name: nameNode(GraphQLDeprecatedDirective.name),
396+
...(change.meta.deprecationReason &&
397+
change.meta.deprecationReason !== DEPRECATION_REASON_DEFAULT
398+
? {
399+
arguments: [
400+
{
401+
kind: Kind.ARGUMENT,
402+
name: nameNode('reason'),
403+
value: stringNode(change.meta.deprecationReason),
404+
},
405+
],
406+
}
407+
: {}),
408+
} as DirectiveNode;
399409

400-
(fieldNode.directives as DirectiveNode[] | undefined) = [
401-
...(fieldNode.directives ?? []),
402-
directiveNode,
403-
];
404-
nodeByPath.set([change.path, `@${GraphQLDeprecatedDirective.name}`].join(','), directiveNode);
410+
(fieldNode.directives as DirectiveNode[] | undefined) = [
411+
...(fieldNode.directives ?? []),
412+
directiveNode,
413+
];
414+
nodeByPath.set(change.path, directiveNode);
415+
}
405416
}
406417
}
407418
}
@@ -411,16 +422,24 @@ export function fieldDeprecationRemoved(
411422
nodeByPath: Map<string, ASTNode>,
412423
config: PatchConfig,
413424
) {
414-
const fieldNode = getChangedNodeOfKind(change, nodeByPath, Kind.FIELD_DEFINITION, config);
415-
if (fieldNode) {
416-
const hasExistingDeprecationDirective = getDeprecatedDirectiveNode(fieldNode);
417-
if (hasExistingDeprecationDirective) {
418-
(fieldNode.directives as DirectiveNode[] | undefined) = fieldNode.directives?.filter(
419-
d => d.name.value !== GraphQLDeprecatedDirective.name,
420-
);
421-
nodeByPath.delete([change.path, `@${GraphQLDeprecatedDirective.name}`].join('.'));
422-
} else {
423-
handleError(change, new DeletedCoordinateNotFound(Kind.DIRECTIVE, '@deprecated'), config);
425+
if (assertChangeHasPath(change, config)) {
426+
const fieldNode = getDeletedParentNodeOfKind(
427+
change,
428+
nodeByPath,
429+
Kind.FIELD_DEFINITION,
430+
'directives',
431+
config,
432+
);
433+
if (fieldNode) {
434+
const hasExistingDeprecationDirective = getDeprecatedDirectiveNode(fieldNode);
435+
if (hasExistingDeprecationDirective) {
436+
(fieldNode.directives as DirectiveNode[] | undefined) = fieldNode.directives?.filter(
437+
d => d.name.value !== GraphQLDeprecatedDirective.name,
438+
);
439+
nodeByPath.delete(change.path);
440+
} else {
441+
handleError(change, new DeletedCoordinateNotFound(Kind.DIRECTIVE, '@deprecated'), config);
442+
}
424443
}
425444
}
426445
}

‎packages/patch/src/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { AdditionChangeType, PatchConfig } from './types.js';
2323
export function getDeprecatedDirectiveNode(
2424
definitionNode: Maybe<{ readonly directives?: ReadonlyArray<DirectiveNode> }>,
2525
): Maybe<DirectiveNode> {
26-
return findNamedNode(definitionNode?.directives, `@${GraphQLDeprecatedDirective.name}`);
26+
return findNamedNode(definitionNode?.directives, GraphQLDeprecatedDirective.name);
2727
}
2828

2929
export function findNamedNode<T extends { readonly name: NameNode }>(

0 commit comments

Comments
 (0)