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. #2894

Closed
Closed
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
110 changes: 110 additions & 0 deletions src/jsutils/__tests__/instanceOf-test.js
Expand Up @@ -2,6 +2,14 @@ import { expect } from 'chai';
import { describe, it } from 'mocha';

import { instanceOf } from '../instanceOf';
import {
GraphQLScalarType,
GraphQLObjectType,
GraphQLInterfaceType,
GraphQLUnionType,
GraphQLEnumType,
GraphQLInputObjectType,
} from '../../type/definition';

describe('instanceOf', () => {
it('fails with descriptive error message', () => {
Expand All @@ -19,4 +27,106 @@ describe('instanceOf', () => {
/^Cannot use Foo "\[object Object\]" from another module or realm./m,
);
});

describe('Symbol.toStringTag', () => {
function checkSameNameClasses(getClass) {
const Class1 = getClass('FirstClass');
const Class2 = getClass('SecondClass');

expect(Class1.name).to.equal('Foo');
expect(Class2.name).to.equal('Foo');

expect(instanceOf(null, Class1)).to.equal(false);
expect(instanceOf(null, Class2)).to.equal(false);

const c1 = new Class1();
const c2 = new Class2();

expect(getTag(c1)).to.equal('FirstClass');
expect(getTag(c2)).to.equal('SecondClass');

// In these Symbol.toStringTag tests, instanceOf returns the
// expected boolean value without throwing an error, because even
// though Class1.name === Class2.name, the Symbol.toStringTag
// strings of the two classes are different.
expect(instanceOf(c1, Class1)).to.equal(true);
expect(instanceOf(c1, Class2)).to.equal(false);
expect(instanceOf(c2, Class1)).to.equal(false);
expect(instanceOf(c2, Class2)).to.equal(true);
}

function getTag(from: any): string {
return from[Symbol.toStringTag];
}

it('does not fail if dynamically-defined tags differ', () => {
checkSameNameClasses((tag) => {
class Foo {}
Object.defineProperty(Foo.prototype, Symbol.toStringTag, {
value: tag,
});
return Foo;
});
});

it('does not fail if dynamically-defined tag getters differ', () => {
checkSameNameClasses((tag) => {
class Foo {}
Object.defineProperty(Foo.prototype, Symbol.toStringTag, {
get() {
return tag;
},
});
return Foo;
});
});

it('does not fail for anonymous classes', () => {
checkSameNameClasses((tag) => {
const Foo = class {};
Object.defineProperty(Foo.prototype, Symbol.toStringTag, {
get() {
return tag;
},
});
return Foo;
});
});

it('does not fail if prototype property tags differ', () => {
checkSameNameClasses((tag) => {
class Foo {}
(Foo.prototype: any)[Symbol.toStringTag] = tag;
return Foo;
});
});

it('does not fail if computed getter tags differ', () => {
checkSameNameClasses((tag) => {
class Foo {
// $FlowFixMe[unsupported-syntax] Flow doesn't support computed properties yet
get [Symbol.toStringTag]() {
return tag;
}
}
return Foo;
});
});

it('is defined for various GraphQL*Type classes', () => {
function checkGraphQLType(constructor, expectedName) {
expect(getTag(constructor.prototype)).to.equal(expectedName);
const instance = Object.create(constructor.prototype);
expect(getTag(instance)).to.equal(expectedName);
expect(instanceOf(instance, constructor)).to.equal(true);
}

checkGraphQLType(GraphQLScalarType, 'GraphQLScalarType');
checkGraphQLType(GraphQLObjectType, 'GraphQLObjectType');
checkGraphQLType(GraphQLInterfaceType, 'GraphQLInterfaceType');
checkGraphQLType(GraphQLUnionType, 'GraphQLUnionType');
checkGraphQLType(GraphQLEnumType, 'GraphQLEnumType');
checkGraphQLType(GraphQLInputObjectType, 'GraphQLInputObjectType');
});
});
});
21 changes: 18 additions & 3 deletions src/jsutils/instanceOf.js
Expand Up @@ -15,9 +15,24 @@ export const instanceOf: (mixed, mixed) => boolean =
return true;
}
if (value) {
const valueClass = value.constructor;
const className = constructor.name;
if (className && valueClass && valueClass.name === className) {
const proto = constructor && constructor.prototype;
const classTag = proto && proto[Symbol.toStringTag];
const className = classTag || constructor.name;
// When the constructor class defines a Symbol.toStringTag
// property, as most classes exported by graphql-js do, use it
// instead of constructor.name and value.constructor.name to
// detect module/realm duplication, since the Symbol.toStringTag
// string is immune to minification. This code runs only when
// process.env.NODE_ENV !== 'production', but minification is
// often enabled in non-production environments like 'staging'.
// In these environments, this error can be thrown mistakenly if
// we rely on constructor.name and value.constructor.name, since
// they could be minified to the same short string, even though
// value is legitimately _not_ instanceof constructor.
const valueName = classTag
? value[Symbol.toStringTag]
: value.constructor && value.constructor.name;
if (typeof className === 'string' && valueName === className) {
throw new Error(
`Cannot use ${className} "${value}" from another module or realm.

Expand Down