Remove DEPRECATE__create method (and fixes related to it)

Summary: This diff contains multiple changes: - Breaking changes to the TestSchema from `relay-test-utils-internal`. Currently, it's an instance of `GraphQLSchema`. This diff makes it an instance of `Schema` - internal schema representation. - Removing `DEPRECATE__create` method from `Schema` module. This method used to create an instance of `GraphQLSchema` to create an instance of `Schema`. We were using this in most of our tests. - Update all tests, where we use to have `DEPRECATE__create`. - `Schema.extend(...)` method can also accept an arrow of strings (with schema extensions) - for more ergonomic usage in the tests. One thing to notice here: - Since now we only accept the `SDL` file as an input for the schema, we no longer able to use `EnumType.getValues()` - which may have different values than the keys (not an issue for internal users) - but may be a problem for OSS customers. Reviewed By: kassens Differential Revision: D18337577 fbshipit-source-id: 32d714b0a3310d55964ebd86524478b49dc72340
facebook · Nov 8, 2019 · 860c23c · 860c23c
1 parent 85db9a1
commit 860c23c
Show file tree

Hide file tree

Showing 42 changed files with 172 additions and 442 deletions.
diff --git a/packages/relay-compiler/codegen/__tests__/compileRelayArtifacts-test.js b/packages/relay-compiler/codegen/__tests__/compileRelayArtifacts-test.js
@@ -11,11 +11,9 @@
 
 'use strict';
 
-const ASTConvert = require('../../core/ASTConvert');
 const CodeMarker = require('../../util/CodeMarker');
 const CompilerContext = require('../../core/CompilerContext');
 const RelayIRTransforms = require('../../core/RelayIRTransforms');
-const Schema = require('../../core/Schema');
 
 const compileRelayArtifacts = require('../compileRelayArtifacts');
 
@@ -39,14 +37,9 @@ describe('compileRelayArtifacts', () => {
   generateTestsFromFixtures(
     `${__dirname}/fixtures/compileRelayArtifacts`,
     text => {
-      const relaySchema = ASTConvert.transformASTSchema(
-        TestSchema,
-        RelayIRTransforms.schemaExtensions,
-      );
+      const relaySchema = TestSchema.extend(RelayIRTransforms.schemaExtensions);
       const {definitions, schema} = parseGraphQLText(relaySchema, text);
-      const compilerContext = new CompilerContext(
-        Schema.DEPRECATED__create(TestSchema, schema),
-      ).addAll(definitions);
+      const compilerContext = new CompilerContext(schema).addAll(definitions);
       return compileRelayArtifacts(compilerContext, RelayIRTransforms)
         .map(([_definition, node]) => {
           if (node.kind === 'Request') {

diff --git a/packages/relay-compiler/core/Schema.js b/packages/relay-compiler/core/Schema.js
@@ -1379,14 +1379,13 @@ class Schema {
     return !this.isServerDefinedField(type, field);
   }
 
-  /**
-   * The only consumer of this is RelayParser.parse(...)
-   * We should either refactor RelayParser.parse(...) to not-rely on the `
-   * extendSchema` method. Or actually implement it here.
-   */
-  extend(document: DocumentNode): Schema {
+  extend(extensions: DocumentNode | $ReadOnlyArray<string>): Schema {
+    const doc = Array.isArray(extensions)
+      ? parse(extensions.join('\n'))
+      : extensions;
+
     // TODO T24511737 figure out if this is dangerous
-    const extendedSchema = extendSchema(this._extendedSchema, document, {
+    const extendedSchema = extendSchema(this._extendedSchema, doc, {
       assumeValid: true,
     });
     return new Schema(this._baseSchema, extendedSchema);
@@ -1416,17 +1415,6 @@ function DEPRECATED__buildGraphQLSchema(source: Source): GraphQLSchema {
     });
   }
 }
-/**
- * We need this in order to make unit-test works
- * In most of the unit-tests the schema is created from the instance of the
- * GraphQLSchema.
- */
-function DEPRECATED__create(
-  baseSchema: GraphQLSchema,
-  extendedSchema: ?GraphQLSchema,
-): Schema {
-  return new Schema(baseSchema, extendedSchema ?? baseSchema);
-}
 
 function create(
   baseSchema: Source,
@@ -1447,6 +1435,5 @@ function create(
 
 module.exports = {
   DEPRECATED__buildGraphQLSchema,
-  DEPRECATED__create,
   create,
 };
diff --git a/packages/relay-compiler/core/__tests__/CompilerContext-test.js b/packages/relay-compiler/core/__tests__/CompilerContext-test.js
@@ -11,7 +11,7 @@
 
 'use strict';
 
-const Schema = require('../Schema');
+const {TestSchema} = require('relay-test-utils-internal');
 
 describe('CompilerContext', () => {
   let CompilerContext;
@@ -20,29 +20,25 @@ describe('CompilerContext', () => {
   let queryFoo;
   let fragmentBar;
   let fragmentFoo;
-  let compilerSchema;
-  let TestSchema;
 
   beforeEach(() => {
     jest.resetModules();
     CompilerContext = require('../CompilerContext');
     RelayParser = require('../RelayParser');
-    ({TestSchema} = require('relay-test-utils-internal'));
-    compilerSchema = Schema.DEPRECATED__create(TestSchema);
   });
 
   describe('add()', () => {
     it('adds multiple roots', () => {
       [queryFoo, fragmentBar] = RelayParser.parse(
-        compilerSchema,
+        TestSchema,
         `
           query Foo { node(id: 1) { ...Bar } }
           fragment Bar on Node { id }
         `,
       );
       const context = [queryFoo, fragmentBar].reduce(
         (ctx, node) => ctx.add(node),
-        new CompilerContext(compilerSchema),
+        new CompilerContext(TestSchema),
       );
 
       expect(context.getRoot('Foo')).toBe(queryFoo);
@@ -51,22 +47,22 @@ describe('CompilerContext', () => {
 
     it('throws if the document names are not unique', () => {
       [queryFoo, fragmentBar] = RelayParser.parse(
-        compilerSchema,
+        TestSchema,
         `
           query Foo { node(id: 1) { ...Bar } }
           fragment Bar on Node { id }
         `,
       );
       [fragmentFoo] = RelayParser.parse(
-        compilerSchema,
+        TestSchema,
         `
           fragment Foo on Node { id }
         `,
       );
       expect(() => {
         [queryFoo, fragmentBar, fragmentFoo].reduce(
           (ctx, node) => ctx.add(node),
-          new CompilerContext(compilerSchema),
+          new CompilerContext(TestSchema),
         );
       }).toThrowError(
         'CompilerContext: Duplicate document named `Foo`. GraphQL ' +

diff --git a/packages/relay-compiler/core/__tests__/IRTransformer-test.js b/packages/relay-compiler/core/__tests__/IRTransformer-test.js
@@ -13,17 +13,13 @@
 
 const CompilerContext = require('../CompilerContext');
 const IRTransformer = require('../IRTransformer');
-const Schema = require('../Schema');
 
-const {transformASTSchema} = require('../ASTConvert');
 const {TestSchema, parseGraphQLText} = require('relay-test-utils-internal');
 
 describe('IRTransformer', () => {
   it('visits all node types', () => {
-    const {definitions} = parseGraphQLText(
-      transformASTSchema(TestSchema, [
-        'directive @test on FRAGMENT_DEFINITION',
-      ]),
+    const {definitions, schema: extendedSchema} = parseGraphQLText(
+      TestSchema.extend(['directive @test on FRAGMENT_DEFINITION']),
       `
    query TestQuery($id: ID!, $condition: Boolean = false) {
      node(id: $id) {
@@ -65,9 +61,7 @@ describe('IRTransformer', () => {
    }
  `,
     );
-    const context = new CompilerContext(
-      Schema.DEPRECATED__create(TestSchema),
-    ).addAll(definitions);
+    const context = new CompilerContext(extendedSchema).addAll(definitions);
 
     const astKinds = [
       'Argument',

diff --git a/packages/relay-compiler/core/__tests__/IRValidator-test.js b/packages/relay-compiler/core/__tests__/IRValidator-test.js
@@ -14,17 +14,13 @@
 const CompilerContext = require('../CompilerContext');
 const IRTransformer = require('../IRTransformer');
 const IRValidator = require('../IRValidator');
-const Schema = require('../Schema');
 
-const {transformASTSchema} = require('../ASTConvert');
 const {TestSchema, parseGraphQLText} = require('relay-test-utils-internal');
 
 describe('IRValidator', () => {
   it('should have same behavior as the IRTransformer', () => {
-    const {definitions} = parseGraphQLText(
-      transformASTSchema(TestSchema, [
-        'directive @test on FRAGMENT_DEFINITION',
-      ]),
+    const {definitions, schema: extendedSchema} = parseGraphQLText(
+      TestSchema.extend(['directive @test on FRAGMENT_DEFINITION']),
       `
    query TestQuery($id: ID!, $condition: Boolean = false) {
      node(id: $id) {
@@ -66,9 +62,7 @@ describe('IRValidator', () => {
    }
  `,
     );
-    const context = new CompilerContext(
-      Schema.DEPRECATED__create(TestSchema),
-    ).addAll(definitions);
+    const context = new CompilerContext(extendedSchema).addAll(definitions);
 
     const astKinds = [
       'Argument',

diff --git a/packages/relay-compiler/core/__tests__/IRVisitor-test.js b/packages/relay-compiler/core/__tests__/IRVisitor-test.js
@@ -13,7 +13,6 @@
 
 const IRPrinter = require('../IRPrinter');
 const RelayParser = require('../RelayParser');
-const Schema = require('../Schema');
 
 const {visit} = require('../IRVisitor');
 const {
@@ -44,22 +43,20 @@ type VisitNodeWithName =
   | Directive
   | ArgumentDefinition;
 
-const schema = Schema.DEPRECATED__create(TestSchema);
-
 describe('IRVisitor', () => {
   generateTestsFromFixtures(
     `${__dirname}/fixtures/visitor/no-op-visit`,
     text => {
-      const ast = RelayParser.parse(schema, text);
+      const ast = RelayParser.parse(TestSchema, text);
       const sameAst = ast.map(fragment => visit(fragment, {}));
-      return sameAst.map(doc => IRPrinter.print(schema, doc)).join('\n');
+      return sameAst.map(doc => IRPrinter.print(TestSchema, doc)).join('\n');
     },
   );
 
   generateTestsFromFixtures(
     `${__dirname}/fixtures/visitor/mutate-visit`,
     text => {
-      const ast = RelayParser.parse(schema, text);
+      const ast = RelayParser.parse(TestSchema, text);
       const mutateNameVisitor = {
         leave: (node: VisitNodeWithName) => {
           return {
@@ -132,7 +129,7 @@ describe('IRVisitor', () => {
         }),
       );
 
-      return mutatedAst.map(doc => IRPrinter.print(schema, doc)).join('\n');
+      return mutatedAst.map(doc => IRPrinter.print(TestSchema, doc)).join('\n');
     },
   );
 });
diff --git a/packages/relay-compiler/core/__tests__/RelayCompilerScope-test.js b/packages/relay-compiler/core/__tests__/RelayCompilerScope-test.js
@@ -10,13 +10,11 @@
 
 'use strict';
 
-const Schema = require('../Schema');
-
 const {getRootScope, getFragmentScope} = require('../RelayCompilerScope');
 const {TestSchema} = require('relay-test-utils-internal');
 
 describe('scope', () => {
-  const schema = Schema.DEPRECATED__create(TestSchema);
+  const schema = TestSchema;
 
   const optionalIntType = schema.expectIntType();
   const requiredIntType = schema.getNonNullType(schema.expectIntType());

diff --git a/packages/relay-compiler/core/__tests__/RelayParser-test.js b/packages/relay-compiler/core/__tests__/RelayParser-test.js
@@ -11,10 +11,8 @@
 
 'use strict';
 
-const ASTConvert = require('../ASTConvert');
 const MatchTransform = require('../../transforms/MatchTransform');
 const RelayParser = require('../RelayParser');
-const Schema = require('../Schema');
 
 const {
   TestSchema,
@@ -23,17 +21,10 @@ const {
 } = require('relay-test-utils-internal');
 
 describe('RelayParser', () => {
-  const schema = Schema.DEPRECATED__create(
-    TestSchema,
-    ASTConvert.transformASTSchema(TestSchema, [
-      MatchTransform.SCHEMA_EXTENSION,
-    ]),
-  );
-
+  const schema = TestSchema.extend([MatchTransform.SCHEMA_EXTENSION]);
   /**
    * Regression tests for T24258497
    */
-
   it("should correctly parse query when input is a non-null type and it's passed to calls expecting both null and non-null types, regardless of order", () => {
     let text;
     // Should work with the call requiring an ID! placed first in the query

diff --git a/packages/relay-compiler/core/__tests__/RelayPrinter-test.js b/packages/relay-compiler/core/__tests__/RelayPrinter-test.js
@@ -14,7 +14,6 @@
 const CompilerContext = require('../CompilerContext');
 const IRPrinter = require('../IRPrinter');
 const RelayParser = require('../RelayParser');
-const Schema = require('../Schema');
 
 const {
   TestSchema,
@@ -23,12 +22,11 @@ const {
 
 describe('IRPrinter', () => {
   generateTestsFromFixtures(`${__dirname}/fixtures/printer`, text => {
-    const compilerSchema = Schema.DEPRECATED__create(TestSchema);
-    const ast = RelayParser.parse(compilerSchema, text);
-    const context = new CompilerContext(compilerSchema).addAll(ast);
+    const ast = RelayParser.parse(TestSchema, text);
+    const context = new CompilerContext(TestSchema).addAll(ast);
     const documents = [];
     context.forEachDocument(doc => {
-      documents.push(IRPrinter.print(compilerSchema, doc));
+      documents.push(IRPrinter.print(TestSchema, doc));
     });
     return documents.join('\n');
   });

diff --git a/packages/relay-compiler/core/__tests__/filterContextForNode-test.js b/packages/relay-compiler/core/__tests__/filterContextForNode-test.js
@@ -13,7 +13,6 @@
 
 const CompilerContext = require('../CompilerContext');
 const IRPrinter = require('../IRPrinter');
-const Schema = require('../Schema');
 
 const filterContextForNode = require('../filterContextForNode');
 
@@ -28,17 +27,16 @@ const MAIN_QUERY_NAME = 'MainQuery';
 describe('filterContextForNode', () => {
   generateTestsFromFixtures(`${__dirname}/fixtures/filter-context`, text => {
     const {definitions} = parseGraphQLText(TestSchema, text);
-    const compilerSchema = Schema.DEPRECATED__create(TestSchema);
 
-    const context = new CompilerContext(compilerSchema).addAll(definitions);
+    const context = new CompilerContext(TestSchema).addAll(definitions);
     const printerContext = filterContextForNode(
       // $FlowFixMe - null or undefined is incompatible with union type
       context.get(MAIN_QUERY_NAME),
       context,
     );
     return printerContext
       .documents()
-      .map(doc => IRPrinter.print(compilerSchema, doc))
+      .map(doc => IRPrinter.print(TestSchema, doc))
       .join('\n');
   });
 });