Skip to content

Commit 213b703

Browse files
committed
Avoid relying on constructor.name for instanceOf error check.
An improvement beyond #1174, for non-production environments that enable minification, such as NODE_ENV='staging'. Since graphql-js goes to the trouble of providing a Symbol.toStringTag property for most of the classes it exports, and that string is immune to minification (unlike constructor.name), we should use it for error checking in instanceOf(value, constructor) whenever the constructor provides a tag, falling back to constructor.name and value.constructor.name for constructors that do not define Symbol.toStringTag (as before). Motivating issue/investigation: apollographql/apollo-client#7446 (comment)
1 parent d720b5b commit 213b703

File tree

2 files changed

+131
-3
lines changed

2 files changed

+131
-3
lines changed

src/jsutils/__tests__/instanceOf-test.js

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,15 @@ import { expect } from 'chai';
22
import { describe, it } from 'mocha';
33

44
import { instanceOf } from '../instanceOf';
5+
import { SYMBOL_TO_STRING_TAG } from '../../polyfills/symbols';
6+
import {
7+
GraphQLScalarType,
8+
GraphQLObjectType,
9+
GraphQLInterfaceType,
10+
GraphQLUnionType,
11+
GraphQLEnumType,
12+
GraphQLInputObjectType,
13+
} from '../../type/definition';
514

615
describe('instanceOf', () => {
716
it('fails with descriptive error message', () => {
@@ -19,4 +28,106 @@ describe('instanceOf', () => {
1928
/^Cannot use Foo "\[object Object\]" from another module or realm./m,
2029
);
2130
});
31+
32+
describe('Symbol.toStringTag', () => {
33+
function checkSameNameClasses(getClass) {
34+
const Class1 = getClass('FirstClass');
35+
const Class2 = getClass('SecondClass');
36+
37+
expect(Class1.name).to.equal('Foo');
38+
expect(Class2.name).to.equal('Foo');
39+
40+
expect(instanceOf(null, Class1)).to.equal(false);
41+
expect(instanceOf(null, Class2)).to.equal(false);
42+
43+
const c1 = new Class1();
44+
const c2 = new Class2();
45+
46+
expect(getTag(c1)).to.equal('FirstClass');
47+
expect(getTag(c2)).to.equal('SecondClass');
48+
49+
// In these Symbol.toStringTag tests, instanceOf returns the
50+
// expected boolean value without throwing an error, because even
51+
// though Class1.name === Class2.name, the Symbol.toStringTag
52+
// strings of the two classes are different.
53+
expect(instanceOf(c1, Class1)).to.equal(true);
54+
expect(instanceOf(c1, Class2)).to.equal(false);
55+
expect(instanceOf(c2, Class1)).to.equal(false);
56+
expect(instanceOf(c2, Class2)).to.equal(true);
57+
}
58+
59+
function getTag(from: any): string {
60+
return from[SYMBOL_TO_STRING_TAG];
61+
}
62+
63+
it('does not fail if dynamically-defined tags differ', () => {
64+
checkSameNameClasses((tag) => {
65+
class Foo {}
66+
Object.defineProperty(Foo.prototype, SYMBOL_TO_STRING_TAG, {
67+
value: tag,
68+
});
69+
return Foo;
70+
});
71+
});
72+
73+
it('does not fail if dynamically-defined tag getters differ', () => {
74+
checkSameNameClasses((tag) => {
75+
class Foo {}
76+
Object.defineProperty(Foo.prototype, SYMBOL_TO_STRING_TAG, {
77+
get() {
78+
return tag;
79+
},
80+
});
81+
return Foo;
82+
});
83+
});
84+
85+
it('does not fail for anonymous classes', () => {
86+
checkSameNameClasses((tag) => {
87+
const Foo = class {};
88+
Object.defineProperty(Foo.prototype, SYMBOL_TO_STRING_TAG, {
89+
get() {
90+
return tag;
91+
},
92+
});
93+
return Foo;
94+
});
95+
});
96+
97+
it('does not fail if prototype property tags differ', () => {
98+
checkSameNameClasses((tag) => {
99+
class Foo {}
100+
(Foo.prototype: any)[SYMBOL_TO_STRING_TAG] = tag;
101+
return Foo;
102+
});
103+
});
104+
105+
it('does not fail if computed getter tags differ', () => {
106+
checkSameNameClasses((tag) => {
107+
class Foo {
108+
// $FlowFixMe[unsupported-syntax] Flow doesn't support computed properties yet
109+
get [SYMBOL_TO_STRING_TAG]() {
110+
return tag;
111+
}
112+
}
113+
return Foo;
114+
});
115+
});
116+
117+
it('is defined for various GraphQL*Type classes', () => {
118+
function checkGraphQLType(constructor, expectedName) {
119+
expect(getTag(constructor.prototype)).to.equal(expectedName);
120+
const instance = Object.create(constructor.prototype);
121+
expect(getTag(instance)).to.equal(expectedName);
122+
expect(instanceOf(instance, constructor)).to.equal(true);
123+
}
124+
125+
checkGraphQLType(GraphQLScalarType, 'GraphQLScalarType');
126+
checkGraphQLType(GraphQLObjectType, 'GraphQLObjectType');
127+
checkGraphQLType(GraphQLInterfaceType, 'GraphQLInterfaceType');
128+
checkGraphQLType(GraphQLUnionType, 'GraphQLUnionType');
129+
checkGraphQLType(GraphQLEnumType, 'GraphQLEnumType');
130+
checkGraphQLType(GraphQLInputObjectType, 'GraphQLInputObjectType');
131+
});
132+
});
22133
});

src/jsutils/instanceOf.js

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { SYMBOL_TO_STRING_TAG } from '../polyfills/symbols';
2+
13
/**
24
* A replacement for instanceof which includes an error warning when multi-realm
35
* constructors are detected.
@@ -15,9 +17,24 @@ export const instanceOf: (mixed, mixed) => boolean =
1517
return true;
1618
}
1719
if (value) {
18-
const valueClass = value.constructor;
19-
const className = constructor.name;
20-
if (className && valueClass && valueClass.name === className) {
20+
const proto = constructor && constructor.prototype;
21+
const classTag = proto && proto[SYMBOL_TO_STRING_TAG];
22+
const className = classTag || constructor.name;
23+
// When the constructor class defines a Symbol.toStringTag
24+
// property, as most classes exported by graphql-js do, use it
25+
// instead of constructor.name and value.constructor.name to
26+
// detect module/realm duplication, since the Symbol.toStringTag
27+
// string is immune to minification. This code runs only when
28+
// process.env.NODE_ENV !== 'production', but minification is
29+
// often enabled in non-production environments like 'staging'.
30+
// In these environments, this error can be thrown mistakenly if
31+
// we rely on constructor.name and value.constructor.name, since
32+
// they could be minified to the same short string, even though
33+
// value is legitimately _not_ instanceof constructor.
34+
const valueName = classTag
35+
? value[SYMBOL_TO_STRING_TAG]
36+
: value.constructor && value.constructor.name;
37+
if (typeof className === 'string' && valueName === className) {
2138
throw new Error(
2239
`Cannot use ${className} "${value}" from another module or realm.
2340

0 commit comments

Comments
 (0)