Repository: graphql/graphql-js
Pull request #3618 (2022-05-30 13:25:21)

extendSchema: Do not modify standard directives
Without this change:
```
const extendedSchema = extendSchema(schema, parse(extensionSDL));
extendSchema.getDirective('skip') === GraphQLSkipDirective
// false
```

Maybe in future we need more comprehensive solution, but for now it's enouch to just match behaviour that we already have to standard types.

yaacovCR: CI broken :(

Change looks ok but somewhat unnecessary as recreated specified directive objects are identical to old directives, only necessary if for example schema modifiers are checking for specified directives via object identity rather than name, which should probably be avoided, as this "mistake" might be made by others.

For example. mapSchema from graphql tools recreates specified directives...
IvanGoncharov: > CI broken :(

Fixed, you can now rerun CI on your PRs. 
IvanGoncharov: > Change looks ok but somewhat unnecessary as recreated specified directive objects are identical to old directives, only necessary if for example schema modifiers are checking for specified directives via object identity rather than name, which should probably be avoided, as this "mistake" might be made by others.

Yes, but it would be a global, even in our own code:
https://github.com/graphql/graphql-js/blob/main/src/utilities/astFromValue.ts#L128
IvanGoncharov: > > CI broken :(
> 
> Fixed, you can now rerun CI on your PRs.

Sadly, GitHub UI doesn't allow to rerun it.
So I just force pushed to trigger it.
yaacovCR: Uncovered problems with #3611 ?
yaacovCR: CI working again -- if you want me to rebase on main and merge, just let me know.

Patch:
diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts
index 998847e9..c451e1e6 100644
--- a/src/utilities/extendSchema.ts
+++ b/src/utilities/extendSchema.ts
@@ -67,6 +67,7 @@ import {
   GraphQLDeprecatedDirective,
   GraphQLDirective,
   GraphQLSpecifiedByDirective,
+  isSpecifiedDirective,
 } from '../type/directives';
 import { introspectionTypes, isIntrospectionType } from '../type/introspection';
 import { isSpecifiedScalarType, specifiedScalarTypes } from '../type/scalars';
@@ -236,6 +237,11 @@ export function extendSchemaImpl(
   }
 
   function replaceDirective(directive: GraphQLDirective): GraphQLDirective {
+    if (isSpecifiedDirective(directive)) {
+      // Builtin directives are not extended.
+      return directive;
+    }
+
     const config = directive.toConfig();
     return new GraphQLDirective({
       ...config,


Test patch:
diff --git a/src/utilities/__tests__/extendSchema-test.ts b/src/utilities/__tests__/extendSchema-test.ts
index 2f4f80e6..c275c023 100644
--- a/src/utilities/__tests__/extendSchema-test.ts
+++ b/src/utilities/__tests__/extendSchema-test.ts
@@ -17,7 +17,7 @@ import {
   assertScalarType,
   assertUnionType,
 } from '../../type/definition';
-import { assertDirective } from '../../type/directives';
+import { assertDirective, specifiedDirectives } from '../../type/directives';
 import {
   GraphQLBoolean,
   GraphQLFloat,
@@ -85,6 +85,34 @@ describe('extendSchema', () => {
     });
   });
 
+  it('Do not modify built-in types and directives', () => {
+    const schema = buildSchema(`
+      type Query {
+        str: String
+        int: Int
+        float: Float
+        id: ID
+        bool: Boolean
+      }
+    `);
+
+    const extensionSDL = dedent`
+      extend type Query {
+        foo: String
+      }
+    `;
+    const extendedSchema = extendSchema(schema, parse(extensionSDL));
+
+    // Built-ins are used
+    expect(extendedSchema.getType('Int')).to.equal(GraphQLInt);
+    expect(extendedSchema.getType('Float')).to.equal(GraphQLFloat);
+    expect(extendedSchema.getType('String')).to.equal(GraphQLString);
+    expect(extendedSchema.getType('Boolean')).to.equal(GraphQLBoolean);
+    expect(extendedSchema.getType('ID')).to.equal(GraphQLID);
+
+    expect(extendedSchema.getDirectives()).to.have.members(specifiedDirectives);
+  });
+
   it('extends objects by adding new fields', () => {
     const schema = buildSchema(`
       type Query {