Skip to content

Commit 51b59cf

Browse files
committed
predicates: use brand check in production
1 parent 51b9794 commit 51b59cf

File tree

8 files changed

+114
-52
lines changed

8 files changed

+114
-52
lines changed

src/jsutils/__tests__/instanceOf-test.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@ describe('instanceOf', () => {
1111
}
1212
}
1313

14-
expect(instanceOf(true, Foo)).to.equal(false);
15-
expect(instanceOf(null, Foo)).to.equal(false);
16-
expect(instanceOf(Object.create(null), Foo)).to.equal(false);
14+
expect(instanceOf(true, true, Foo)).to.equal(false);
15+
expect(instanceOf(false, true, Foo)).to.equal(false);
16+
expect(instanceOf(true, null, Foo)).to.equal(false);
17+
expect(instanceOf(false, null, Foo)).to.equal(false);
18+
expect(instanceOf(true, Object.create(null), Foo)).to.equal(false);
19+
expect(instanceOf(false, Object.create(null), Foo)).to.equal(false);
1720
});
1821

1922
it('detect name clashes with older versions of this lib', () => {
@@ -33,8 +36,10 @@ describe('instanceOf', () => {
3336

3437
const NewClass = newVersion();
3538
const OldClass = oldVersion();
36-
expect(instanceOf(new NewClass(), NewClass)).to.equal(true);
37-
expect(() => instanceOf(new OldClass(), NewClass)).to.throw();
39+
expect(instanceOf(true, new NewClass(), NewClass)).to.equal(true);
40+
expect(instanceOf(false, new NewClass(), NewClass)).to.equal(false);
41+
expect(() => instanceOf(true, new OldClass(), NewClass)).to.throw();
42+
expect(instanceOf(false, new OldClass(), NewClass)).to.equal(false);
3843
});
3944

4045
it('allows instances to have share the same constructor name', () => {
@@ -49,12 +54,16 @@ describe('instanceOf', () => {
4954

5055
const Foo = getMinifiedClass('Foo');
5156
const Bar = getMinifiedClass('Bar');
52-
expect(instanceOf(new Foo(), Bar)).to.equal(false);
53-
expect(instanceOf(new Bar(), Foo)).to.equal(false);
57+
expect(instanceOf(true, new Foo(), Bar)).to.equal(false);
58+
expect(instanceOf(false, new Foo(), Bar)).to.equal(false);
59+
expect(instanceOf(true, new Bar(), Foo)).to.equal(false);
60+
expect(instanceOf(false, new Bar(), Foo)).to.equal(false);
5461

5562
const DuplicateOfFoo = getMinifiedClass('Foo');
56-
expect(() => instanceOf(new DuplicateOfFoo(), Foo)).to.throw();
57-
expect(() => instanceOf(new Foo(), DuplicateOfFoo)).to.throw();
63+
expect(() => instanceOf(true, new DuplicateOfFoo(), Foo)).to.throw();
64+
expect(instanceOf(false, new DuplicateOfFoo(), Foo)).to.equal(false);
65+
expect(() => instanceOf(true, new Foo(), DuplicateOfFoo)).to.throw();
66+
expect(instanceOf(false, new Foo(), DuplicateOfFoo)).to.equal(false);
5867
});
5968

6069
it('fails with descriptive error message', () => {
@@ -69,11 +78,13 @@ describe('instanceOf', () => {
6978
const Foo1 = getFoo();
7079
const Foo2 = getFoo();
7180

72-
expect(() => instanceOf(new Foo1(), Foo2)).to.throw(
81+
expect(() => instanceOf(true, new Foo1(), Foo2)).to.throw(
7382
/^Cannot use Foo "{}" from another module or realm./m,
7483
);
75-
expect(() => instanceOf(new Foo2(), Foo1)).to.throw(
84+
expect(instanceOf(false, new Foo1(), Foo2)).to.equal(false);
85+
expect(() => instanceOf(true, new Foo2(), Foo1)).to.throw(
7686
/^Cannot use Foo "{}" from another module or realm./m,
7787
);
88+
expect(instanceOf(false, new Foo2(), Foo1)).to.equal(false);
7889
});
7990
});

src/jsutils/instanceOf.ts

Lines changed: 29 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { inspect } from './inspect.js';
2+
import { invariant } from './invariant.js';
23

34
/* c8 ignore next 3 */
45
const isProduction =
@@ -7,34 +8,42 @@ const isProduction =
78
process.env.NODE_ENV === 'production';
89

910
/**
10-
* A replacement for instanceof which includes an error warning when multi-realm
11-
* constructors are detected.
11+
* A utility which throws an error warning when multi-realm constructors are detected,
12+
* triggered by a positive brand check and a negative instanceof (for an object).
13+
*
14+
* When not in development mode, only the brand check is used.
15+
*
1216
* See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production
1317
* See: https://webpack.js.org/guides/production/
1418
*/
15-
export const instanceOf: (value: unknown, constructor: Constructor) => boolean =
16-
/* c8 ignore next 6 */
19+
export const instanceOf: (
20+
brandCheck: true | undefined,
21+
value: unknown,
22+
constructor: Constructor,
23+
) => boolean =
24+
/* c8 ignore next 9 */
1725
// FIXME: https://github.com/graphql/graphql-js/issues/2317
1826
isProduction
19-
? function instanceOf(value: unknown, constructor: Constructor): boolean {
20-
return value instanceof constructor;
27+
? function instanceOf(brandCheck: true | undefined): boolean {
28+
return brandCheck === true;
2129
}
22-
: function instanceOf(value: unknown, constructor: Constructor): boolean {
30+
: function instanceOf(
31+
brandCheck: true | undefined,
32+
value: unknown,
33+
constructor: Constructor,
34+
): boolean {
35+
if (!brandCheck) {
36+
return false;
37+
}
2338
if (value instanceof constructor) {
2439
return true;
2540
}
26-
if (typeof value === 'object' && value !== null) {
27-
// Prefer Symbol.toStringTag since it is immune to minification.
28-
const className = constructor.prototype[Symbol.toStringTag];
29-
const valueClassName =
30-
// We still need to support constructor's name to detect conflicts with older versions of this library.
31-
Symbol.toStringTag in value
32-
? value[Symbol.toStringTag]
33-
: value.constructor?.name;
34-
if (className === valueClassName) {
35-
const stringifiedValue = inspect(value);
36-
throw new Error(
37-
`Cannot use ${className} "${stringifiedValue}" from another module or realm.
41+
invariant(typeof value === 'object' && value !== null);
42+
// Prefer Symbol.toStringTag since it is immune to minification.
43+
const className = constructor.prototype[Symbol.toStringTag];
44+
const stringifiedValue = inspect(value);
45+
throw new Error(
46+
`Cannot use ${className} "${stringifiedValue}" from another module or realm.
3847
3948
Ensure that there is only one instance of "graphql" in the node_modules
4049
directory. If different versions of "graphql" are the dependencies of other
@@ -46,10 +55,7 @@ Duplicate "graphql" modules cannot be used at the same time since different
4655
versions may have different capabilities and behavior. The data from one
4756
version used in the function from another could produce confusing and
4857
spurious results.`,
49-
);
50-
}
51-
}
52-
return false;
58+
);
5359
};
5460

5561
interface Constructor {

src/language/source.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ interface Location {
1414
* The `line` and `column` properties in `locationOffset` are 1-indexed.
1515
*/
1616
export class Source {
17+
readonly __isSource = true as const;
18+
1719
body: string;
1820
name: string;
1921
locationOffset: Location;
@@ -47,5 +49,5 @@ export class Source {
4749
* @internal
4850
*/
4951
export function isSource(source: unknown): source is Source {
50-
return instanceOf(source, Source);
52+
return instanceOf((source as any)?.__isSource, source, Source);
5153
}

0 commit comments

Comments
 (0)