Fix the .extend method

Reviewed By: kassens Differential Revision: D18352623 fbshipit-source-id: be505c531e957ab0615410c8c8d4d11ed5788ea1
facebook · Nov 7, 2019 · 2fa5aa0 · 2fa5aa0
1 parent 6ee64ae
commit 2fa5aa0
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 78 deletions.
diff --git a/packages/relay-compiler/core/RelayParser.js b/packages/relay-compiler/core/RelayParser.js
@@ -133,12 +133,7 @@ function parse(
   filename?: string,
 ): $ReadOnlyArray<Root | Fragment> {
   const ast = parseGraphQL(new Source(text, filename));
-
-  // TODO T24511737 figure out if this is dangerous
-  const parser = new RelayParser(
-    schema.DEPRECATED__extend(ast),
-    ast.definitions,
-  );
+  const parser = new RelayParser(schema.extend(ast), ast.definitions);
   return parser.transform();
 }
 

diff --git a/packages/relay-compiler/core/Schema.js b/packages/relay-compiler/core/Schema.js
@@ -359,49 +359,38 @@ class Schema {
   /**
    * @private
    */
-  constructor(
-    baseSchema: GraphQLSchema,
-    extendedSchema: GraphQLSchema,
-    typeMap: TypeMap,
-    fieldsMap: Map<Type, FieldsMap>,
-    typeNameMap: Map<Type, Field>,
-    clientIdMap: Map<Type, Field>,
-    possibleTypesMap: Map<AbstractTypeID, Set<CompositeTypeID>>,
-    directivesMap: Map<string, Directive> | null,
-  ) {
+  constructor(baseSchema: GraphQLSchema, extendedSchema: GraphQLSchema) {
     this._baseSchema = baseSchema;
     this._extendedSchema = extendedSchema;
-    this._typeMap = typeMap;
-    this._fieldsMap = fieldsMap;
-    this._typeNameMap = typeNameMap;
-    this._clientIdMap = clientIdMap;
-    this._possibleTypesMap = possibleTypesMap;
-    this._directivesMap =
-      directivesMap ??
-      new Map(
-        this._extendedSchema.getDirectives().map(directive => {
-          return [
-            directive.name,
-            {
-              clientOnlyDirective:
-                this._baseSchema.getDirective(directive.name) == null,
-              name: directive.name,
-              locations: directive.locations,
-              args: directive.args.map(arg => {
-                return {
-                  name: arg.name,
-                  type: this.assertInputType(
-                    arg.astNode
-                      ? this.expectTypeFromAST(arg.astNode.type)
-                      : this.expectTypeFromString(String(arg.type)),
-                  ),
-                  defaultValue: arg.defaultValue,
-                };
-              }),
-            },
-          ];
-        }),
-      );
+    this._typeMap = new Map();
+    this._fieldsMap = new Map();
+    this._typeNameMap = new Map();
+    this._clientIdMap = new Map();
+    this._possibleTypesMap = new Map();
+    this._directivesMap = new Map(
+      this._extendedSchema.getDirectives().map(directive => {
+        return [
+          directive.name,
+          {
+            clientOnlyDirective:
+              this._baseSchema.getDirective(directive.name) == null,
+            name: directive.name,
+            locations: directive.locations,
+            args: directive.args.map(arg => {
+              return {
+                name: arg.name,
+                type: this.assertInputType(
+                  arg.astNode
+                    ? this.expectTypeFromAST(arg.astNode.type)
+                    : this.expectTypeFromString(String(arg.type)),
+                ),
+                defaultValue: arg.defaultValue,
+              };
+            }),
+          },
+        ];
+      }),
+    );
   }
 
   getTypeFromAST(typeNode: TypeNode): ?TypeID {
@@ -1395,20 +1384,12 @@ class Schema {
    * We should either refactor RelayParser.parse(...) to not-rely on the `
    * extendSchema` method. Or actually implement it here.
    */
-  DEPRECATED__extend(document: DocumentNode): Schema {
+  extend(document: DocumentNode): Schema {
+    // TODO T24511737 figure out if this is dangerous
     const extendedSchema = extendSchema(this._extendedSchema, document, {
       assumeValid: true,
     });
-    return new Schema(
-      this._baseSchema,
-      extendedSchema,
-      this._typeMap,
-      this._fieldsMap,
-      this._typeNameMap,
-      this._clientIdMap,
-      this._possibleTypesMap,
-      this._directivesMap,
-    );
+    return new Schema(this._baseSchema, extendedSchema);
   }
 }
 
@@ -1444,16 +1425,7 @@ function DEPRECATED__create(
   baseSchema: GraphQLSchema,
   extendedSchema: ?GraphQLSchema,
 ): Schema {
-  return new Schema(
-    baseSchema,
-    extendedSchema ?? baseSchema,
-    new Map(),
-    new Map(),
-    new Map(),
-    new Map(),
-    new Map(),
-    null,
-  );
+  return new Schema(baseSchema, extendedSchema ?? baseSchema);
 }
 
 function create(
@@ -1470,16 +1442,7 @@ function create(
     transformedSchema,
     schemaExtensionDocuments ?? [],
   );
-  return new Schema(
-    schema,
-    extendedSchema,
-    new Map(),
-    new Map(),
-    new Map(),
-    new Map(),
-    new Map(),
-    null,
-  );
+  return new Schema(schema, extendedSchema);
 }
 
 module.exports = {

diff --git a/packages/relay-compiler/core/__tests__/Schema-test.js b/packages/relay-compiler/core/__tests__/Schema-test.js
@@ -13,6 +13,8 @@
 
 const Schema = require('../Schema');
 
+const nullthrows = require('../../util/nullthrowsOSS');
+
 const {Source, parse, parseType} = require('graphql');
 
 describe('Schema: RelayCompiler Internal GraphQL Schema Interface', () => {
@@ -1358,4 +1360,39 @@ describe('Schema: RelayCompiler Internal GraphQL Schema Interface', () => {
     expect(schema.doTypesOverlap(actorUser, actor)).toBe(true);
     expect(schema.doTypesOverlap(actorUser, node)).toBe(true);
   });
+
+  test('extend', () => {
+    let schema = Schema.create(
+      new Source(`
+        directive @my_server_directive on QUERY
+        type User {
+          name: String
+        }
+    `),
+    );
+    expect(schema.getDirective('my_server_directive')).toBeDefined();
+    expect(schema.getDirective('my_client_directive')).not.toBeDefined();
+    schema = schema.extend(
+      parse(`
+      directive @my_client_directive on QUERY
+      type ClientType {
+        value: String
+      }
+      extend type User {
+        lastName: String
+      }
+    `),
+    );
+    expect(schema.getDirective('my_client_directive')).toBeDefined();
+    const user = schema.assertCompositeType(
+      schema.expectTypeFromString('User'),
+    );
+    expect(
+      schema.isServerField(nullthrows(schema.getFieldByName(user, 'name'))),
+    ).toBe(true);
+    expect(
+      schema.isServerField(nullthrows(schema.getFieldByName(user, 'lastName'))),
+    ).toBe(false);
+    expect(schema.getTypeFromString('ClientType')).toBeDefined();
+  });
 });