From 2fa5aa05b2118994e6f8ae32b91b40cd8dce53b9 Mon Sep 17 00:00:00 2001 From: Andrey Lunyov Date: Thu, 7 Nov 2019 11:05:16 -0800 Subject: [PATCH] Fix the .extend method Reviewed By: kassens Differential Revision: D18352623 fbshipit-source-id: be505c531e957ab0615410c8c8d4d11ed5788ea1 --- packages/relay-compiler/core/RelayParser.js | 7 +- packages/relay-compiler/core/Schema.js | 107 ++++++------------ .../core/__tests__/Schema-test.js | 37 ++++++ 3 files changed, 73 insertions(+), 78 deletions(-) diff --git a/packages/relay-compiler/core/RelayParser.js b/packages/relay-compiler/core/RelayParser.js index e71f6efad1aac..b9e9257fa85be 100644 --- a/packages/relay-compiler/core/RelayParser.js +++ b/packages/relay-compiler/core/RelayParser.js @@ -133,12 +133,7 @@ function parse( filename?: string, ): $ReadOnlyArray { 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 index 7501d4eb3e8f2..969202afffe11 100644 --- 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, - typeNameMap: Map, - clientIdMap: Map, - possibleTypesMap: Map>, - directivesMap: Map | 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 index 3359528b67234..59eb4ba701b9d 100644 --- 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(); + }); });