Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid relying on constructor.name for instanceOf error check. #3172

Merged
merged 1 commit into from Jun 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 27 additions & 3 deletions src/jsutils/__tests__/instanceOf-test.ts
Expand Up @@ -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,
);
});
});
25 changes: 18 additions & 7 deletions 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.
Expand All @@ -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
Expand All @@ -38,5 +47,7 @@ spurious results.`,
};

interface Constructor extends Function {
name: string;
prototype: {
[Symbol.toStringTag]: string;
};
}