From 58122ef35f08175c8d054de427b49f570901536e Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 13 Jun 2021 20:25:50 +0300 Subject: [PATCH] Avoid relying on constructor.name for instanceOf error check. (#3172) Fixes #2894 --- src/jsutils/__tests__/instanceOf-test.ts | 30 +++++++++++++++++++++--- src/jsutils/instanceOf.ts | 25 ++++++++++++++------ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/jsutils/__tests__/instanceOf-test.ts b/src/jsutils/__tests__/instanceOf-test.ts index dfa364c9ce..5acfab52d9 100644 --- a/src/jsutils/__tests__/instanceOf-test.ts +++ b/src/jsutils/__tests__/instanceOf-test.ts @@ -4,19 +4,43 @@ import { describe, it } from 'mocha'; import { instanceOf } from '../instanceOf'; describe('instanceOf', () => { + it('allows instances to have share the same constructor name', () => { + function getMinifiedClass(tag: string) { + class SomeNameAfterMinification { + get [Symbol.toStringTag]() { + return tag; + } + } + return SomeNameAfterMinification; + } + + 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 DuplicateOfFoo = getMinifiedClass('Foo'); + expect(() => instanceOf(new DuplicateOfFoo(), Foo)).to.throw(); + expect(() => instanceOf(new Foo(), DuplicateOfFoo)).to.throw(); + }); + it('fails with descriptive error message', () => { function getFoo() { - class Foo {} + class Foo { + get [Symbol.toStringTag]() { + return 'Foo'; + } + } return Foo; } const Foo1 = getFoo(); const Foo2 = getFoo(); expect(() => instanceOf(new Foo1(), Foo2)).to.throw( - /^Cannot use Foo "\[object Object\]" from another module or realm./m, + /^Cannot use Foo "{}" from another module or realm./m, ); expect(() => instanceOf(new Foo2(), Foo1)).to.throw( - /^Cannot use Foo "\[object Object\]" from another module or realm./m, + /^Cannot use Foo "{}" from another module or realm./m, ); }); }); diff --git a/src/jsutils/instanceOf.ts b/src/jsutils/instanceOf.ts index 26b6c16d1b..e403fa50ec 100644 --- a/src/jsutils/instanceOf.ts +++ b/src/jsutils/instanceOf.ts @@ -1,3 +1,5 @@ +import { inspect } from './inspect'; + /** * A replacement for instanceof which includes an error warning when multi-realm * constructors are detected. @@ -10,16 +12,23 @@ export const instanceOf: (value: unknown, constructor: Constructor) => boolean = function instanceOf(value: unknown, constructor: Constructor): boolean { return value instanceof constructor; } - : function instanceOf(value: any, constructor: Constructor): boolean { + : function instanceOf(value: unknown, constructor: Constructor): boolean { if (value instanceof constructor) { return true; } - if (value) { - const valueClass = value.constructor; - const className = constructor.name; - if (className && valueClass && valueClass.name === className) { + if (typeof value === 'object' && value !== null) { + // Prefer Symbol.toStringTag since it is immune to minification. + const className = constructor.prototype[Symbol.toStringTag]; + const valueClassName = + // We still need to support constructor's name to detect conflicts with older versions of this library. + Symbol.toStringTag in value + ? // @ts-expect-error TS bug see, https://github.com/microsoft/TypeScript/issues/38009 + value[Symbol.toStringTag] + : value.constructor?.name; + if (className === valueClassName) { + const stringifiedValue = inspect(value); throw new Error( - `Cannot use ${className} "${value}" from another module or realm. + `Cannot use ${className} "${stringifiedValue}" from another module or realm. Ensure that there is only one instance of "graphql" in the node_modules directory. If different versions of "graphql" are the dependencies of other @@ -38,5 +47,7 @@ spurious results.`, }; interface Constructor extends Function { - name: string; + prototype: { + [Symbol.toStringTag]: string; + }; }