From 62d545a2f437c5dfa42d48e2721bd75d953acfca Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Wed, 6 Aug 2025 01:44:44 +0300 Subject: [PATCH 1/4] predicates: use brand check in production --- src/jsutils/__tests__/instanceOf-test.ts | 43 ++++++++++++++------ src/jsutils/instanceOf.ts | 25 ++++++++---- src/language/source.ts | 4 +- src/type/definition.ts | 52 ++++++++++++++++++------ src/type/directives.ts | 8 +++- src/type/schema.ts | 4 +- src/utilities/extendSchema.ts | 5 ++- src/utilities/typeFromAST.ts | 8 +++- 8 files changed, 111 insertions(+), 38 deletions(-) diff --git a/src/jsutils/__tests__/instanceOf-test.ts b/src/jsutils/__tests__/instanceOf-test.ts index 5a54a641e5..9971937bee 100644 --- a/src/jsutils/__tests__/instanceOf-test.ts +++ b/src/jsutils/__tests__/instanceOf-test.ts @@ -6,14 +6,15 @@ import { instanceOf } from '../instanceOf.js'; describe('instanceOf', () => { it('do not throw on values without prototype', () => { class Foo { + readonly __isFoo = true as const; get [Symbol.toStringTag]() { return 'Foo'; } } - expect(instanceOf(true, Foo)).to.equal(false); - expect(instanceOf(null, Foo)).to.equal(false); - expect(instanceOf(Object.create(null), Foo)).to.equal(false); + expect(instanceOf(undefined, true, Foo)).to.equal(false); + expect(instanceOf(undefined, null, Foo)).to.equal(false); + expect(instanceOf(undefined, Object.create(null), Foo)).to.equal(false); }); it('detect name clashes with older versions of this lib', () => { @@ -24,6 +25,7 @@ describe('instanceOf', () => { function newVersion() { class Foo { + readonly __isFoo = true as const; get [Symbol.toStringTag]() { return 'Foo'; } @@ -33,13 +35,17 @@ describe('instanceOf', () => { const NewClass = newVersion(); const OldClass = oldVersion(); - expect(instanceOf(new NewClass(), NewClass)).to.equal(true); - expect(() => instanceOf(new OldClass(), NewClass)).to.throw(); + const newInstance = new NewClass(); + expect(instanceOf(newInstance.__isFoo, newInstance, NewClass)).to.equal( + true, + ); + expect(() => instanceOf(undefined, new OldClass(), NewClass)).to.throw(); }); it('allows instances to have share the same constructor name', () => { function getMinifiedClass(tag: string) { class SomeNameAfterMinification { + readonly [tag] = true as const; get [Symbol.toStringTag]() { return tag; } @@ -49,17 +55,25 @@ describe('instanceOf', () => { const Foo = getMinifiedClass('Foo'); const Bar = getMinifiedClass('Bar'); - expect(instanceOf(new Foo(), Bar)).to.equal(false); - expect(instanceOf(new Bar(), Foo)).to.equal(false); + const fooInstance = new Foo(); + const barInstance = new Bar(); + expect(instanceOf(fooInstance.foo, fooInstance, Bar)).to.equal(false); + expect(instanceOf(barInstance.bar, barInstance, Foo)).to.equal(false); const DuplicateOfFoo = getMinifiedClass('Foo'); - expect(() => instanceOf(new DuplicateOfFoo(), Foo)).to.throw(); - expect(() => instanceOf(new Foo(), DuplicateOfFoo)).to.throw(); + const duplicateOfFooInstance = new DuplicateOfFoo(); + expect(() => + instanceOf(duplicateOfFooInstance.foo, new DuplicateOfFoo(), Foo), + ).to.throw(); + expect(() => + instanceOf(fooInstance.foo, fooInstance, DuplicateOfFoo), + ).to.throw(); }); it('fails with descriptive error message', () => { function getFoo() { class Foo { + readonly __isFoo = true as const; get [Symbol.toStringTag]() { return 'Foo'; } @@ -69,11 +83,14 @@ describe('instanceOf', () => { const Foo1 = getFoo(); const Foo2 = getFoo(); - expect(() => instanceOf(new Foo1(), Foo2)).to.throw( - /^Cannot use Foo "{}" from another module or realm./m, + const foo1Instance = new Foo1(); + const foo2Instance = new Foo2(); + + expect(() => instanceOf(foo1Instance.__isFoo, foo1Instance, Foo2)).to.throw( + /^Cannot use Foo "{ __isFoo: true }" from another module or realm./m, ); - expect(() => instanceOf(new Foo2(), Foo1)).to.throw( - /^Cannot use Foo "{}" from another module or realm./m, + expect(() => instanceOf(foo2Instance.__isFoo, foo2Instance, Foo1)).to.throw( + /^Cannot use Foo "{ __isFoo: true }" from another module or realm./m, ); }); }); diff --git a/src/jsutils/instanceOf.ts b/src/jsutils/instanceOf.ts index 66811433ae..b223d16031 100644 --- a/src/jsutils/instanceOf.ts +++ b/src/jsutils/instanceOf.ts @@ -7,19 +7,30 @@ const isProduction = process.env.NODE_ENV === 'production'; /** - * A replacement for instanceof which includes an error warning when multi-realm - * constructors are detected. + * A utility which throws an error warning when multi-realm constructors are detected, + * triggered by a positive brand check and a negative instanceof (for an object). + * + * When not in development mode, only the brand check is used. + * * See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production * See: https://webpack.js.org/guides/production/ */ -export const instanceOf: (value: unknown, constructor: Constructor) => boolean = - /* c8 ignore next 6 */ +export const instanceOf: ( + brandCheck: true | undefined, + value: unknown, + constructor: Constructor, +) => boolean = + /* c8 ignore next 9 */ // FIXME: https://github.com/graphql/graphql-js/issues/2317 isProduction - ? function instanceOf(value: unknown, constructor: Constructor): boolean { - return value instanceof constructor; + ? function instanceOf(brandCheck: true | undefined): boolean { + return brandCheck === true; } - : function instanceOf(value: unknown, constructor: Constructor): boolean { + : function instanceOf( + _brandCheck: true | undefined, + value: unknown, + constructor: Constructor, + ): boolean { if (value instanceof constructor) { return true; } diff --git a/src/language/source.ts b/src/language/source.ts index eb21547154..ff84ab6ced 100644 --- a/src/language/source.ts +++ b/src/language/source.ts @@ -14,6 +14,8 @@ interface Location { * The `line` and `column` properties in `locationOffset` are 1-indexed. */ export class Source { + readonly __isSource = true as const; + body: string; name: string; locationOffset: Location; @@ -47,5 +49,5 @@ export class Source { * @internal */ export function isSource(source: unknown): source is Source { - return instanceOf(source, Source); + return instanceOf((source as any)?.__isSource, source, Source); } diff --git a/src/type/definition.ts b/src/type/definition.ts index 1bbce5df5e..8a0f3012b8 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -80,7 +80,7 @@ export function assertType(type: unknown): GraphQLType { * There are predicates for each GraphQL schema element. */ export function isScalarType(type: unknown): type is GraphQLScalarType { - return instanceOf(type, GraphQLScalarType); + return instanceOf((type as any)?.__isScalar, type, GraphQLScalarType); } export function assertScalarType(type: unknown): GraphQLScalarType { @@ -91,7 +91,7 @@ export function assertScalarType(type: unknown): GraphQLScalarType { } export function isObjectType(type: unknown): type is GraphQLObjectType { - return instanceOf(type, GraphQLObjectType); + return instanceOf((type as any)?.__isObject, type, GraphQLObjectType); } export function assertObjectType(type: unknown): GraphQLObjectType { @@ -102,7 +102,7 @@ export function assertObjectType(type: unknown): GraphQLObjectType { } export function isField(field: unknown): field is GraphQLField { - return instanceOf(field, GraphQLField); + return instanceOf((field as any)?.__isField, field, GraphQLField); } export function assertField(field: unknown): GraphQLField { @@ -113,7 +113,7 @@ export function assertField(field: unknown): GraphQLField { } export function isArgument(arg: unknown): arg is GraphQLArgument { - return instanceOf(arg, GraphQLArgument); + return instanceOf((arg as any)?.__isArgument, arg, GraphQLArgument); } export function assertArgument(arg: unknown): GraphQLArgument { @@ -124,7 +124,7 @@ export function assertArgument(arg: unknown): GraphQLArgument { } export function isInterfaceType(type: unknown): type is GraphQLInterfaceType { - return instanceOf(type, GraphQLInterfaceType); + return instanceOf((type as any)?.__isInterface, type, GraphQLInterfaceType); } export function assertInterfaceType(type: unknown): GraphQLInterfaceType { @@ -137,7 +137,7 @@ export function assertInterfaceType(type: unknown): GraphQLInterfaceType { } export function isUnionType(type: unknown): type is GraphQLUnionType { - return instanceOf(type, GraphQLUnionType); + return instanceOf((type as any)?.__isUnion, type, GraphQLUnionType); } export function assertUnionType(type: unknown): GraphQLUnionType { @@ -148,7 +148,7 @@ export function assertUnionType(type: unknown): GraphQLUnionType { } export function isEnumType(type: unknown): type is GraphQLEnumType { - return instanceOf(type, GraphQLEnumType); + return instanceOf((type as any)?.__isEnum, type, GraphQLEnumType); } export function assertEnumType(type: unknown): GraphQLEnumType { @@ -159,7 +159,7 @@ export function assertEnumType(type: unknown): GraphQLEnumType { } export function isEnumValue(value: unknown): value is GraphQLEnumValue { - return instanceOf(value, GraphQLEnumValue); + return instanceOf((value as any)?.__isEnumValue, value, GraphQLEnumValue); } export function assertEnumValue(value: unknown): GraphQLEnumValue { @@ -172,7 +172,11 @@ export function assertEnumValue(value: unknown): GraphQLEnumValue { export function isInputObjectType( type: unknown, ): type is GraphQLInputObjectType { - return instanceOf(type, GraphQLInputObjectType); + return instanceOf( + (type as any)?.__isInputObject, + type, + GraphQLInputObjectType, + ); } export function assertInputObjectType(type: unknown): GraphQLInputObjectType { @@ -185,7 +189,7 @@ export function assertInputObjectType(type: unknown): GraphQLInputObjectType { } export function isInputField(field: unknown): field is GraphQLInputField { - return instanceOf(field, GraphQLInputField); + return instanceOf((field as any)?.__isInputField, field, GraphQLInputField); } export function assertInputField(field: unknown): GraphQLInputField { @@ -203,7 +207,7 @@ export function isListType( ): type is GraphQLList; export function isListType(type: unknown): type is GraphQLList; export function isListType(type: unknown): type is GraphQLList { - return instanceOf(type, GraphQLList); + return instanceOf((type as any)?.__isList, type, GraphQLList); } export function assertListType(type: unknown): GraphQLList { @@ -225,7 +229,7 @@ export function isNonNullType( export function isNonNullType( type: unknown, ): type is GraphQLNonNull { - return instanceOf(type, GraphQLNonNull); + return instanceOf((type as any)?.__isNonNull, type, GraphQLNonNull); } export function assertNonNullType( @@ -368,9 +372,11 @@ export function assertAbstractType(type: unknown): GraphQLAbstractType { export class GraphQLList implements GraphQLSchemaElement { + readonly __isList: true; readonly ofType: T; constructor(ofType: T) { + this.__isList = true; this.ofType = ofType; } @@ -411,9 +417,11 @@ export class GraphQLList export class GraphQLNonNull implements GraphQLSchemaElement { + readonly __isNonNull: true; readonly ofType: T; constructor(ofType: T) { + this.__isNonNull = true; this.ofType = ofType; } @@ -651,6 +659,7 @@ export interface GraphQLScalarTypeExtensions { export class GraphQLScalarType implements GraphQLSchemaElement { + readonly __isScalar: true; name: string; description: Maybe; specifiedByURL: Maybe; @@ -669,6 +678,7 @@ export class GraphQLScalarType extensionASTNodes: ReadonlyArray; constructor(config: Readonly>) { + this.__isScalar = true; this.name = assertName(config.name); this.description = config.description; this.specifiedByURL = config.specifiedByURL; @@ -869,6 +879,7 @@ export interface GraphQLObjectTypeExtensions<_TSource = any, _TContext = any> { export class GraphQLObjectType implements GraphQLSchemaElement { + readonly __isObject = true; name: string; description: Maybe; isTypeOf: Maybe>; @@ -880,6 +891,7 @@ export class GraphQLObjectType private _interfaces: ThunkReadonlyArray; constructor(config: Readonly>) { + this.__isObject = true; this.name = assertName(config.name); this.description = config.description; this.isTypeOf = config.isTypeOf; @@ -1093,6 +1105,7 @@ export type GraphQLFieldNormalizedConfigMap = ObjMap< export class GraphQLField implements GraphQLSchemaElement { + readonly __isField = true; parentType: | GraphQLObjectType | GraphQLInterfaceType @@ -1115,6 +1128,7 @@ export class GraphQLField name: string, config: GraphQLFieldConfig, ) { + this.__isField = true; this.parentType = parentType; this.name = assertName(name); this.description = config.description; @@ -1166,6 +1180,7 @@ export class GraphQLField } export class GraphQLArgument implements GraphQLSchemaElement { + readonly __isArgument: true; parent: GraphQLField | GraphQLDirective; name: string; description: Maybe; @@ -1181,6 +1196,7 @@ export class GraphQLArgument implements GraphQLSchemaElement { name: string, config: GraphQLArgumentConfig, ) { + this.__isArgument = true; this.parent = parent; this.name = assertName(name); this.description = config.description; @@ -1270,6 +1286,7 @@ export interface GraphQLInterfaceTypeExtensions { export class GraphQLInterfaceType implements GraphQLSchemaElement { + readonly __isInterface: true; name: string; description: Maybe; resolveType: Maybe>; @@ -1281,6 +1298,7 @@ export class GraphQLInterfaceType private _interfaces: ThunkReadonlyArray; constructor(config: Readonly>) { + this.__isInterface = true; this.name = assertName(config.name); this.description = config.description; this.resolveType = config.resolveType; @@ -1397,6 +1415,7 @@ export interface GraphQLUnionTypeExtensions { * ``` */ export class GraphQLUnionType implements GraphQLSchemaElement { + readonly __isUnion: true; name: string; description: Maybe; resolveType: Maybe>; @@ -1407,6 +1426,7 @@ export class GraphQLUnionType implements GraphQLSchemaElement { private _types: ThunkReadonlyArray; constructor(config: Readonly>) { + this.__isUnion = true; this.name = assertName(config.name); this.description = config.description; this.resolveType = config.resolveType; @@ -1514,6 +1534,7 @@ export interface GraphQLEnumTypeExtensions { * will be used as its internal value. */ export class GraphQLEnumType /* */ implements GraphQLSchemaElement { + readonly __isEnum: true; name: string; description: Maybe; extensions: Readonly; @@ -1528,6 +1549,7 @@ export class GraphQLEnumType /* */ implements GraphQLSchemaElement { private _nameLookup: ObjMap | null; constructor(config: Readonly */>) { + this.__isEnum = true; this.name = assertName(config.name); this.description = config.description; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1738,6 +1760,7 @@ export interface GraphQLEnumValueNormalizedConfig } export class GraphQLEnumValue implements GraphQLSchemaElement { + readonly __isEnumValue: true; parentEnum: GraphQLEnumType; name: string; description: Maybe; @@ -1751,6 +1774,7 @@ export class GraphQLEnumValue implements GraphQLSchemaElement { name: string, config: GraphQLEnumValueConfig, ) { + this.__isEnumValue = true; this.parentEnum = parentEnum; this.name = assertEnumValueName(name); this.description = config.description; @@ -1818,6 +1842,7 @@ export interface GraphQLInputObjectTypeExtensions { * ``` */ export class GraphQLInputObjectType implements GraphQLSchemaElement { + readonly __isInputObject: true; name: string; description: Maybe; extensions: Readonly; @@ -1828,6 +1853,7 @@ export class GraphQLInputObjectType implements GraphQLSchemaElement { private _fields: ThunkObjMap; constructor(config: Readonly) { + this.__isInputObject = true; this.name = assertName(config.name); this.description = config.description; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1935,6 +1961,7 @@ export type GraphQLInputFieldNormalizedConfigMap = ObjMap; export class GraphQLInputField implements GraphQLSchemaElement { + readonly __isInputField: true; parentType: GraphQLInputObjectType; name: string; description: Maybe; @@ -1955,6 +1982,7 @@ export class GraphQLInputField implements GraphQLSchemaElement { `${parentType}.${name} field has a resolve property, but Input Types cannot define resolvers.`, ); + this.__isInputField = true; this.parentType = parentType; this.name = assertName(name); this.description = config.description; diff --git a/src/type/directives.ts b/src/type/directives.ts index 96f5b6b65a..7c469759ae 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -23,7 +23,11 @@ import { GraphQLBoolean, GraphQLInt, GraphQLString } from './scalars.js'; * Test if the given value is a GraphQL directive. */ export function isDirective(directive: unknown): directive is GraphQLDirective { - return instanceOf(directive, GraphQLDirective); + return instanceOf( + (directive as any)?.__isDirective, + directive, + GraphQLDirective, + ); } export function assertDirective(directive: unknown): GraphQLDirective { @@ -53,6 +57,7 @@ export interface GraphQLDirectiveExtensions { * behavior. Type system creators will usually not create these directly. */ export class GraphQLDirective implements GraphQLSchemaElement { + readonly __isDirective: true; name: string; description: Maybe; locations: ReadonlyArray; @@ -62,6 +67,7 @@ export class GraphQLDirective implements GraphQLSchemaElement { astNode: Maybe; constructor(config: Readonly) { + this.__isDirective = true; this.name = assertName(config.name); this.description = config.description; this.locations = config.locations; diff --git a/src/type/schema.ts b/src/type/schema.ts index 0f19c5998c..4299a985d1 100644 --- a/src/type/schema.ts +++ b/src/type/schema.ts @@ -41,7 +41,7 @@ import { * Test if the given value is a GraphQL schema. */ export function isSchema(schema: unknown): schema is GraphQLSchema { - return instanceOf(schema, GraphQLSchema); + return instanceOf((schema as any)?.__isSchema, schema, GraphQLSchema); } export function assertSchema(schema: unknown): GraphQLSchema { @@ -133,6 +133,7 @@ export interface GraphQLSchemaExtensions { * ``` */ export class GraphQLSchema { + readonly __isSchema: true; description: Maybe; extensions: Readonly; astNode: Maybe; @@ -157,6 +158,7 @@ export class GraphQLSchema { }>; constructor(config: Readonly) { + this.__isSchema = true; // If this schema was built from a source known to be valid, then it may be // marked with assumeValid to avoid an additional type system validation. this.assumeValid = config.assumeValid ?? false; diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 93b3776533..04512399a1 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -34,6 +34,7 @@ import type { GraphQLFieldNormalizedConfigMap, GraphQLInputFieldNormalizedConfigMap, GraphQLNamedType, + GraphQLNullableType, GraphQLType, } from '../type/definition.js'; import { @@ -349,7 +350,9 @@ export function extendSchemaImpl( return new GraphQLList(typeFromAST(node.type)); } if (node.kind === Kind.NON_NULL_TYPE) { - return new GraphQLNonNull(typeFromAST(node.type)); + return new GraphQLNonNull( + typeFromAST(node.type) as GraphQLNullableType, + ); } return namedTypeFromAST(node); } diff --git a/src/utilities/typeFromAST.ts b/src/utilities/typeFromAST.ts index c072301b39..6761294bef 100644 --- a/src/utilities/typeFromAST.ts +++ b/src/utilities/typeFromAST.ts @@ -6,7 +6,11 @@ import type { } from '../language/ast.js'; import { Kind } from '../language/kinds.js'; -import type { GraphQLNamedType, GraphQLType } from '../type/definition.js'; +import type { + GraphQLNamedType, + GraphQLNullableType, + GraphQLType, +} from '../type/definition.js'; import { GraphQLList, GraphQLNonNull } from '../type/definition.js'; import type { GraphQLSchema } from '../type/schema.js'; @@ -44,7 +48,7 @@ export function typeFromAST( } case Kind.NON_NULL_TYPE: { const innerType = typeFromAST(schema, typeNode.type); - return innerType && new GraphQLNonNull(innerType); + return innerType && new GraphQLNonNull(innerType as GraphQLNullableType); } case Kind.NAMED_TYPE: return schema.getType(typeNode.name.value); From a531d6321a88a28bf5ff4c1f6d876c234bbcf3cb Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 7 Aug 2025 05:21:03 +0300 Subject: [PATCH 2/4] revert most of the comment change --- src/jsutils/instanceOf.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/jsutils/instanceOf.ts b/src/jsutils/instanceOf.ts index b223d16031..25c4a7516c 100644 --- a/src/jsutils/instanceOf.ts +++ b/src/jsutils/instanceOf.ts @@ -7,10 +7,10 @@ const isProduction = process.env.NODE_ENV === 'production'; /** - * A utility which throws an error warning when multi-realm constructors are detected, - * triggered by a positive brand check and a negative instanceof (for an object). + * A replacement for instanceof which includes an error warning when multi-realm + * constructors are detected. * - * When not in development mode, only the brand check is used. + * In production, it simply uses the provided brand check value. * * See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production * See: https://webpack.js.org/guides/production/ From cfb5e914b8179eee4cc5ca8cd82ded22caff6142 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 7 Aug 2025 06:56:13 +0300 Subject: [PATCH 3/4] tweak wording --- src/jsutils/instanceOf.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/jsutils/instanceOf.ts b/src/jsutils/instanceOf.ts index 25c4a7516c..502a332c23 100644 --- a/src/jsutils/instanceOf.ts +++ b/src/jsutils/instanceOf.ts @@ -10,24 +10,24 @@ const isProduction = * A replacement for instanceof which includes an error warning when multi-realm * constructors are detected. * - * In production, it simply uses the provided brand check value. + * In production, it simply uses the provided type brand. * * See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production * See: https://webpack.js.org/guides/production/ */ export const instanceOf: ( - brandCheck: true | undefined, + typeBrand: true | undefined, value: unknown, constructor: Constructor, ) => boolean = /* c8 ignore next 9 */ // FIXME: https://github.com/graphql/graphql-js/issues/2317 isProduction - ? function instanceOf(brandCheck: true | undefined): boolean { - return brandCheck === true; + ? function instanceOf(typeBrand: true | undefined): boolean { + return typeBrand === true; } : function instanceOf( - _brandCheck: true | undefined, + _typeBrand: true | undefined, value: unknown, constructor: Constructor, ): boolean { From f81453e6c05ab4ddb6939f17549168890ee845dc Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Fri, 8 Aug 2025 19:46:38 +0300 Subject: [PATCH 4/4] use symbols --- src/jsutils/__tests__/instanceOf-test.ts | 75 ++++++++--------- src/jsutils/instanceOf.ts | 18 ++-- src/language/source.ts | 7 +- src/type/definition.ts | 100 ++++++++++++++--------- src/type/directives.ts | 12 ++- src/type/schema.ts | 7 +- src/utilities/extendSchema.ts | 5 +- src/utilities/typeFromAST.ts | 8 +- 8 files changed, 120 insertions(+), 112 deletions(-) diff --git a/src/jsutils/__tests__/instanceOf-test.ts b/src/jsutils/__tests__/instanceOf-test.ts index 9971937bee..f0920c80bc 100644 --- a/src/jsutils/__tests__/instanceOf-test.ts +++ b/src/jsutils/__tests__/instanceOf-test.ts @@ -5,16 +5,17 @@ import { instanceOf } from '../instanceOf.js'; describe('instanceOf', () => { it('do not throw on values without prototype', () => { + const fooSymbol: unique symbol = Symbol('Foo'); class Foo { - readonly __isFoo = true as const; + readonly __kind: symbol = fooSymbol; get [Symbol.toStringTag]() { return 'Foo'; } } - expect(instanceOf(undefined, true, Foo)).to.equal(false); - expect(instanceOf(undefined, null, Foo)).to.equal(false); - expect(instanceOf(undefined, Object.create(null), Foo)).to.equal(false); + expect(instanceOf(true, fooSymbol, Foo)).to.equal(false); + expect(instanceOf(null, fooSymbol, Foo)).to.equal(false); + expect(instanceOf(Object.create(null), fooSymbol, Foo)).to.equal(false); }); it('detect name clashes with older versions of this lib', () => { @@ -24,73 +25,67 @@ describe('instanceOf', () => { } function newVersion() { - class Foo { - readonly __isFoo = true as const; + const fooSymbol: unique symbol = Symbol('Foo'); + class FooClass { + readonly __kind: symbol = fooSymbol; get [Symbol.toStringTag]() { return 'Foo'; } } - return Foo; + return { fooSymbol, FooClass }; } - const NewClass = newVersion(); + const { fooSymbol: newSymbol, FooClass: NewClass } = newVersion(); const OldClass = oldVersion(); - const newInstance = new NewClass(); - expect(instanceOf(newInstance.__isFoo, newInstance, NewClass)).to.equal( - true, - ); - expect(() => instanceOf(undefined, new OldClass(), NewClass)).to.throw(); + expect(instanceOf(new NewClass(), newSymbol, NewClass)).to.equal(true); + expect(() => instanceOf(new OldClass(), newSymbol, NewClass)).to.throw(); }); it('allows instances to have share the same constructor name', () => { function getMinifiedClass(tag: string) { + const someSymbol: unique symbol = Symbol(tag); class SomeNameAfterMinification { - readonly [tag] = true as const; + readonly __kind: symbol = someSymbol; get [Symbol.toStringTag]() { return tag; } } - return SomeNameAfterMinification; + return { someSymbol, SomeNameAfterMinification }; } - const Foo = getMinifiedClass('Foo'); - const Bar = getMinifiedClass('Bar'); - const fooInstance = new Foo(); - const barInstance = new Bar(); - expect(instanceOf(fooInstance.foo, fooInstance, Bar)).to.equal(false); - expect(instanceOf(barInstance.bar, barInstance, Foo)).to.equal(false); + const { someSymbol: fooSymbol, SomeNameAfterMinification: Foo } = + getMinifiedClass('Foo'); + const { someSymbol: barSymbol, SomeNameAfterMinification: Bar } = + getMinifiedClass('Bar'); + expect(instanceOf(new Foo(), barSymbol, Bar)).to.equal(false); + expect(instanceOf(new Bar(), fooSymbol, Foo)).to.equal(false); - const DuplicateOfFoo = getMinifiedClass('Foo'); - const duplicateOfFooInstance = new DuplicateOfFoo(); - expect(() => - instanceOf(duplicateOfFooInstance.foo, new DuplicateOfFoo(), Foo), - ).to.throw(); - expect(() => - instanceOf(fooInstance.foo, fooInstance, DuplicateOfFoo), - ).to.throw(); + const { + someSymbol: duplicateOfFooSymbol, + SomeNameAfterMinification: DuplicateOfFoo, + } = getMinifiedClass('Foo'); + expect(() => instanceOf(new DuplicateOfFoo(), fooSymbol, Foo)).to.throw(); + expect(() => instanceOf(new Foo(), duplicateOfFooSymbol, Foo)).to.throw(); }); it('fails with descriptive error message', () => { function getFoo() { + const fooSymbol: unique symbol = Symbol('Foo'); class Foo { - readonly __isFoo = true as const; get [Symbol.toStringTag]() { return 'Foo'; } } - return Foo; + return { fooSymbol, Foo }; } - const Foo1 = getFoo(); - const Foo2 = getFoo(); - - const foo1Instance = new Foo1(); - const foo2Instance = new Foo2(); + const { fooSymbol: foo1Symbol, Foo: Foo1 } = getFoo(); + const { fooSymbol: foo2Symbol, Foo: Foo2 } = getFoo(); - expect(() => instanceOf(foo1Instance.__isFoo, foo1Instance, Foo2)).to.throw( - /^Cannot use Foo "{ __isFoo: true }" from another module or realm./m, + expect(() => instanceOf(new Foo1(), foo2Symbol, Foo2)).to.throw( + /^Cannot use Foo "{}" from another module or realm./m, ); - expect(() => instanceOf(foo2Instance.__isFoo, foo2Instance, Foo1)).to.throw( - /^Cannot use Foo "{ __isFoo: true }" from another module or realm./m, + expect(() => instanceOf(new Foo2(), foo1Symbol, Foo1)).to.throw( + /^Cannot use Foo "{}" from another module or realm./m, ); }); }); diff --git a/src/jsutils/instanceOf.ts b/src/jsutils/instanceOf.ts index 502a332c23..2a483ce121 100644 --- a/src/jsutils/instanceOf.ts +++ b/src/jsutils/instanceOf.ts @@ -7,31 +7,29 @@ const isProduction = process.env.NODE_ENV === 'production'; /** - * A replacement for instanceof which includes an error warning when multi-realm - * constructors are detected. - * - * In production, it simply uses the provided type brand. - * + * A replacement for instanceof relying on a symbol-driven type brand which in + * development mode includes an error warning when multi-realm constructors are + * detected. * See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production * See: https://webpack.js.org/guides/production/ */ export const instanceOf: ( - typeBrand: true | undefined, value: unknown, + symbol: symbol, constructor: Constructor, ) => boolean = /* c8 ignore next 9 */ // FIXME: https://github.com/graphql/graphql-js/issues/2317 isProduction - ? function instanceOf(typeBrand: true | undefined): boolean { - return typeBrand === true; + ? function instanceOf(value: unknown, symbol: symbol): boolean { + return (value as any)?.__kind === symbol; } : function instanceOf( - _typeBrand: true | undefined, value: unknown, + symbol: symbol, constructor: Constructor, ): boolean { - if (value instanceof constructor) { + if ((value as any)?.__kind === symbol) { return true; } if (typeof value === 'object' && value !== null) { diff --git a/src/language/source.ts b/src/language/source.ts index ff84ab6ced..06d78b2c3f 100644 --- a/src/language/source.ts +++ b/src/language/source.ts @@ -6,6 +6,8 @@ interface Location { column: number; } +const sourceSymbol: unique symbol = Symbol('Source'); + /** * A representation of source input to GraphQL. The `name` and `locationOffset` parameters are * optional, but they are useful for clients who store GraphQL documents in source files. @@ -14,7 +16,7 @@ interface Location { * The `line` and `column` properties in `locationOffset` are 1-indexed. */ export class Source { - readonly __isSource = true as const; + readonly __kind: symbol; body: string; name: string; @@ -25,6 +27,7 @@ export class Source { name: string = 'GraphQL request', locationOffset: Location = { line: 1, column: 1 }, ) { + this.__kind = sourceSymbol; this.body = body; this.name = name; this.locationOffset = locationOffset; @@ -49,5 +52,5 @@ export class Source { * @internal */ export function isSource(source: unknown): source is Source { - return instanceOf((source as any)?.__isSource, source, Source); + return instanceOf(source, sourceSymbol, Source); } diff --git a/src/type/definition.ts b/src/type/definition.ts index 8a0f3012b8..b35c44947e 100644 --- a/src/type/definition.ts +++ b/src/type/definition.ts @@ -76,11 +76,13 @@ export function assertType(type: unknown): GraphQLType { return type; } +const scalarSymbol: unique symbol = Symbol('Scalar'); + /** * There are predicates for each GraphQL schema element. */ export function isScalarType(type: unknown): type is GraphQLScalarType { - return instanceOf((type as any)?.__isScalar, type, GraphQLScalarType); + return instanceOf(type, scalarSymbol, GraphQLScalarType); } export function assertScalarType(type: unknown): GraphQLScalarType { @@ -90,8 +92,10 @@ export function assertScalarType(type: unknown): GraphQLScalarType { return type; } +const objectSymbol: unique symbol = Symbol('Object'); + export function isObjectType(type: unknown): type is GraphQLObjectType { - return instanceOf((type as any)?.__isObject, type, GraphQLObjectType); + return instanceOf(type, objectSymbol, GraphQLObjectType); } export function assertObjectType(type: unknown): GraphQLObjectType { @@ -101,8 +105,10 @@ export function assertObjectType(type: unknown): GraphQLObjectType { return type; } +const fieldSymbol: unique symbol = Symbol('Field'); + export function isField(field: unknown): field is GraphQLField { - return instanceOf((field as any)?.__isField, field, GraphQLField); + return instanceOf(field, fieldSymbol, GraphQLField); } export function assertField(field: unknown): GraphQLField { @@ -112,8 +118,10 @@ export function assertField(field: unknown): GraphQLField { return field; } +const argumentSymbol: unique symbol = Symbol('Argument'); + export function isArgument(arg: unknown): arg is GraphQLArgument { - return instanceOf((arg as any)?.__isArgument, arg, GraphQLArgument); + return instanceOf(arg, argumentSymbol, GraphQLArgument); } export function assertArgument(arg: unknown): GraphQLArgument { @@ -123,8 +131,10 @@ export function assertArgument(arg: unknown): GraphQLArgument { return arg; } +const interfaceSymbol: unique symbol = Symbol('Interface'); + export function isInterfaceType(type: unknown): type is GraphQLInterfaceType { - return instanceOf((type as any)?.__isInterface, type, GraphQLInterfaceType); + return instanceOf(type, interfaceSymbol, GraphQLInterfaceType); } export function assertInterfaceType(type: unknown): GraphQLInterfaceType { @@ -136,8 +146,10 @@ export function assertInterfaceType(type: unknown): GraphQLInterfaceType { return type; } +const unionSymbol: unique symbol = Symbol('Union'); + export function isUnionType(type: unknown): type is GraphQLUnionType { - return instanceOf((type as any)?.__isUnion, type, GraphQLUnionType); + return instanceOf(type, unionSymbol, GraphQLUnionType); } export function assertUnionType(type: unknown): GraphQLUnionType { @@ -147,8 +159,10 @@ export function assertUnionType(type: unknown): GraphQLUnionType { return type; } +const enumSymbol: unique symbol = Symbol('Enum'); + export function isEnumType(type: unknown): type is GraphQLEnumType { - return instanceOf((type as any)?.__isEnum, type, GraphQLEnumType); + return instanceOf(type, enumSymbol, GraphQLEnumType); } export function assertEnumType(type: unknown): GraphQLEnumType { @@ -158,8 +172,10 @@ export function assertEnumType(type: unknown): GraphQLEnumType { return type; } +const enumValueSymbol: unique symbol = Symbol('EnumValue'); + export function isEnumValue(value: unknown): value is GraphQLEnumValue { - return instanceOf((value as any)?.__isEnumValue, value, GraphQLEnumValue); + return instanceOf(value, enumValueSymbol, GraphQLEnumValue); } export function assertEnumValue(value: unknown): GraphQLEnumValue { @@ -169,14 +185,12 @@ export function assertEnumValue(value: unknown): GraphQLEnumValue { return value; } +const inputObjectSymbol: unique symbol = Symbol('InputObject'); + export function isInputObjectType( type: unknown, ): type is GraphQLInputObjectType { - return instanceOf( - (type as any)?.__isInputObject, - type, - GraphQLInputObjectType, - ); + return instanceOf(type, inputObjectSymbol, GraphQLInputObjectType); } export function assertInputObjectType(type: unknown): GraphQLInputObjectType { @@ -188,8 +202,10 @@ export function assertInputObjectType(type: unknown): GraphQLInputObjectType { return type; } +const inputFieldSymbol: unique symbol = Symbol('InputField'); + export function isInputField(field: unknown): field is GraphQLInputField { - return instanceOf((field as any)?.__isInputField, field, GraphQLInputField); + return instanceOf(field, inputFieldSymbol, GraphQLInputField); } export function assertInputField(field: unknown): GraphQLInputField { @@ -199,6 +215,8 @@ export function assertInputField(field: unknown): GraphQLInputField { return field; } +const listSymbol: unique symbol = Symbol('List'); + export function isListType( type: GraphQLInputType, ): type is GraphQLList; @@ -207,7 +225,7 @@ export function isListType( ): type is GraphQLList; export function isListType(type: unknown): type is GraphQLList; export function isListType(type: unknown): type is GraphQLList { - return instanceOf((type as any)?.__isList, type, GraphQLList); + return instanceOf(type, listSymbol, GraphQLList); } export function assertListType(type: unknown): GraphQLList { @@ -217,6 +235,8 @@ export function assertListType(type: unknown): GraphQLList { return type; } +const nonNullSymbol: unique symbol = Symbol('NonNull'); + export function isNonNullType( type: GraphQLInputType, ): type is GraphQLNonNull; @@ -229,7 +249,7 @@ export function isNonNullType( export function isNonNullType( type: unknown, ): type is GraphQLNonNull { - return instanceOf((type as any)?.__isNonNull, type, GraphQLNonNull); + return instanceOf(type, nonNullSymbol, GraphQLNonNull); } export function assertNonNullType( @@ -372,11 +392,11 @@ export function assertAbstractType(type: unknown): GraphQLAbstractType { export class GraphQLList implements GraphQLSchemaElement { - readonly __isList: true; + readonly __kind: symbol; readonly ofType: T; constructor(ofType: T) { - this.__isList = true; + this.__kind = listSymbol; this.ofType = ofType; } @@ -417,11 +437,11 @@ export class GraphQLList export class GraphQLNonNull implements GraphQLSchemaElement { - readonly __isNonNull: true; + readonly __kind: symbol; readonly ofType: T; constructor(ofType: T) { - this.__isNonNull = true; + this.__kind = nonNullSymbol; this.ofType = ofType; } @@ -659,7 +679,7 @@ export interface GraphQLScalarTypeExtensions { export class GraphQLScalarType implements GraphQLSchemaElement { - readonly __isScalar: true; + readonly __kind: symbol; name: string; description: Maybe; specifiedByURL: Maybe; @@ -678,7 +698,7 @@ export class GraphQLScalarType extensionASTNodes: ReadonlyArray; constructor(config: Readonly>) { - this.__isScalar = true; + this.__kind = scalarSymbol; this.name = assertName(config.name); this.description = config.description; this.specifiedByURL = config.specifiedByURL; @@ -879,7 +899,7 @@ export interface GraphQLObjectTypeExtensions<_TSource = any, _TContext = any> { export class GraphQLObjectType implements GraphQLSchemaElement { - readonly __isObject = true; + readonly __kind = objectSymbol; name: string; description: Maybe; isTypeOf: Maybe>; @@ -891,7 +911,7 @@ export class GraphQLObjectType private _interfaces: ThunkReadonlyArray; constructor(config: Readonly>) { - this.__isObject = true; + this.__kind = objectSymbol; this.name = assertName(config.name); this.description = config.description; this.isTypeOf = config.isTypeOf; @@ -1105,7 +1125,7 @@ export type GraphQLFieldNormalizedConfigMap = ObjMap< export class GraphQLField implements GraphQLSchemaElement { - readonly __isField = true; + readonly __kind: symbol; parentType: | GraphQLObjectType | GraphQLInterfaceType @@ -1128,7 +1148,7 @@ export class GraphQLField name: string, config: GraphQLFieldConfig, ) { - this.__isField = true; + this.__kind = fieldSymbol; this.parentType = parentType; this.name = assertName(name); this.description = config.description; @@ -1180,7 +1200,7 @@ export class GraphQLField } export class GraphQLArgument implements GraphQLSchemaElement { - readonly __isArgument: true; + readonly __kind: symbol; parent: GraphQLField | GraphQLDirective; name: string; description: Maybe; @@ -1196,7 +1216,7 @@ export class GraphQLArgument implements GraphQLSchemaElement { name: string, config: GraphQLArgumentConfig, ) { - this.__isArgument = true; + this.__kind = argumentSymbol; this.parent = parent; this.name = assertName(name); this.description = config.description; @@ -1286,7 +1306,7 @@ export interface GraphQLInterfaceTypeExtensions { export class GraphQLInterfaceType implements GraphQLSchemaElement { - readonly __isInterface: true; + readonly __kind: symbol; name: string; description: Maybe; resolveType: Maybe>; @@ -1298,7 +1318,7 @@ export class GraphQLInterfaceType private _interfaces: ThunkReadonlyArray; constructor(config: Readonly>) { - this.__isInterface = true; + this.__kind = interfaceSymbol; this.name = assertName(config.name); this.description = config.description; this.resolveType = config.resolveType; @@ -1415,7 +1435,7 @@ export interface GraphQLUnionTypeExtensions { * ``` */ export class GraphQLUnionType implements GraphQLSchemaElement { - readonly __isUnion: true; + readonly __kind: symbol; name: string; description: Maybe; resolveType: Maybe>; @@ -1426,7 +1446,7 @@ export class GraphQLUnionType implements GraphQLSchemaElement { private _types: ThunkReadonlyArray; constructor(config: Readonly>) { - this.__isUnion = true; + this.__kind = unionSymbol; this.name = assertName(config.name); this.description = config.description; this.resolveType = config.resolveType; @@ -1534,7 +1554,7 @@ export interface GraphQLEnumTypeExtensions { * will be used as its internal value. */ export class GraphQLEnumType /* */ implements GraphQLSchemaElement { - readonly __isEnum: true; + readonly __kind: symbol; name: string; description: Maybe; extensions: Readonly; @@ -1549,7 +1569,7 @@ export class GraphQLEnumType /* */ implements GraphQLSchemaElement { private _nameLookup: ObjMap | null; constructor(config: Readonly */>) { - this.__isEnum = true; + this.__kind = enumSymbol; this.name = assertName(config.name); this.description = config.description; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1760,7 +1780,7 @@ export interface GraphQLEnumValueNormalizedConfig } export class GraphQLEnumValue implements GraphQLSchemaElement { - readonly __isEnumValue: true; + readonly __kind: symbol; parentEnum: GraphQLEnumType; name: string; description: Maybe; @@ -1774,7 +1794,7 @@ export class GraphQLEnumValue implements GraphQLSchemaElement { name: string, config: GraphQLEnumValueConfig, ) { - this.__isEnumValue = true; + this.__kind = enumValueSymbol; this.parentEnum = parentEnum; this.name = assertEnumValueName(name); this.description = config.description; @@ -1842,7 +1862,7 @@ export interface GraphQLInputObjectTypeExtensions { * ``` */ export class GraphQLInputObjectType implements GraphQLSchemaElement { - readonly __isInputObject: true; + readonly __kind: symbol; name: string; description: Maybe; extensions: Readonly; @@ -1853,7 +1873,7 @@ export class GraphQLInputObjectType implements GraphQLSchemaElement { private _fields: ThunkObjMap; constructor(config: Readonly) { - this.__isInputObject = true; + this.__kind = inputObjectSymbol; this.name = assertName(config.name); this.description = config.description; this.extensions = toObjMapWithSymbols(config.extensions); @@ -1961,7 +1981,7 @@ export type GraphQLInputFieldNormalizedConfigMap = ObjMap; export class GraphQLInputField implements GraphQLSchemaElement { - readonly __isInputField: true; + readonly __kind: symbol; parentType: GraphQLInputObjectType; name: string; description: Maybe; @@ -1982,7 +2002,7 @@ export class GraphQLInputField implements GraphQLSchemaElement { `${parentType}.${name} field has a resolve property, but Input Types cannot define resolvers.`, ); - this.__isInputField = true; + this.__kind = inputFieldSymbol; this.parentType = parentType; this.name = assertName(name); this.description = config.description; diff --git a/src/type/directives.ts b/src/type/directives.ts index 7c469759ae..a3170aed26 100644 --- a/src/type/directives.ts +++ b/src/type/directives.ts @@ -19,15 +19,13 @@ import type { import { GraphQLArgument, GraphQLNonNull } from './definition.js'; import { GraphQLBoolean, GraphQLInt, GraphQLString } from './scalars.js'; +const directiveSymbol: unique symbol = Symbol('Directive'); + /** * Test if the given value is a GraphQL directive. */ export function isDirective(directive: unknown): directive is GraphQLDirective { - return instanceOf( - (directive as any)?.__isDirective, - directive, - GraphQLDirective, - ); + return instanceOf(directive, directiveSymbol, GraphQLDirective); } export function assertDirective(directive: unknown): GraphQLDirective { @@ -57,7 +55,7 @@ export interface GraphQLDirectiveExtensions { * behavior. Type system creators will usually not create these directly. */ export class GraphQLDirective implements GraphQLSchemaElement { - readonly __isDirective: true; + readonly __kind: symbol; name: string; description: Maybe; locations: ReadonlyArray; @@ -67,7 +65,7 @@ export class GraphQLDirective implements GraphQLSchemaElement { astNode: Maybe; constructor(config: Readonly) { - this.__isDirective = true; + this.__kind = directiveSymbol; this.name = assertName(config.name); this.description = config.description; this.locations = config.locations; diff --git a/src/type/schema.ts b/src/type/schema.ts index 4299a985d1..e8600eb980 100644 --- a/src/type/schema.ts +++ b/src/type/schema.ts @@ -41,7 +41,7 @@ import { * Test if the given value is a GraphQL schema. */ export function isSchema(schema: unknown): schema is GraphQLSchema { - return instanceOf((schema as any)?.__isSchema, schema, GraphQLSchema); + return instanceOf(schema, schemaSymbol, GraphQLSchema); } export function assertSchema(schema: unknown): GraphQLSchema { @@ -64,6 +64,8 @@ export interface GraphQLSchemaExtensions { [attributeName: string | symbol]: unknown; } +const schemaSymbol: unique symbol = Symbol('Schema'); + /** * Schema Definition * @@ -133,7 +135,7 @@ export interface GraphQLSchemaExtensions { * ``` */ export class GraphQLSchema { - readonly __isSchema: true; + readonly __kind = schemaSymbol; description: Maybe; extensions: Readonly; astNode: Maybe; @@ -158,7 +160,6 @@ export class GraphQLSchema { }>; constructor(config: Readonly) { - this.__isSchema = true; // If this schema was built from a source known to be valid, then it may be // marked with assumeValid to avoid an additional type system validation. this.assumeValid = config.assumeValid ?? false; diff --git a/src/utilities/extendSchema.ts b/src/utilities/extendSchema.ts index 04512399a1..93b3776533 100644 --- a/src/utilities/extendSchema.ts +++ b/src/utilities/extendSchema.ts @@ -34,7 +34,6 @@ import type { GraphQLFieldNormalizedConfigMap, GraphQLInputFieldNormalizedConfigMap, GraphQLNamedType, - GraphQLNullableType, GraphQLType, } from '../type/definition.js'; import { @@ -350,9 +349,7 @@ export function extendSchemaImpl( return new GraphQLList(typeFromAST(node.type)); } if (node.kind === Kind.NON_NULL_TYPE) { - return new GraphQLNonNull( - typeFromAST(node.type) as GraphQLNullableType, - ); + return new GraphQLNonNull(typeFromAST(node.type)); } return namedTypeFromAST(node); } diff --git a/src/utilities/typeFromAST.ts b/src/utilities/typeFromAST.ts index 6761294bef..c072301b39 100644 --- a/src/utilities/typeFromAST.ts +++ b/src/utilities/typeFromAST.ts @@ -6,11 +6,7 @@ import type { } from '../language/ast.js'; import { Kind } from '../language/kinds.js'; -import type { - GraphQLNamedType, - GraphQLNullableType, - GraphQLType, -} from '../type/definition.js'; +import type { GraphQLNamedType, GraphQLType } from '../type/definition.js'; import { GraphQLList, GraphQLNonNull } from '../type/definition.js'; import type { GraphQLSchema } from '../type/schema.js'; @@ -48,7 +44,7 @@ export function typeFromAST( } case Kind.NON_NULL_TYPE: { const innerType = typeFromAST(schema, typeNode.type); - return innerType && new GraphQLNonNull(innerType as GraphQLNullableType); + return innerType && new GraphQLNonNull(innerType); } case Kind.NAMED_TYPE: return schema.getType(typeNode.name.value);