Skip to content

Commit 62d545a

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

File tree

8 files changed

+111
-38
lines changed

8 files changed

+111
-38
lines changed

src/jsutils/__tests__/instanceOf-test.ts

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@ import { instanceOf } from '../instanceOf.js';
66
describe('instanceOf', () => {
77
it('do not throw on values without prototype', () => {
88
class Foo {
9+
readonly __isFoo = true as const;
910
get [Symbol.toStringTag]() {
1011
return 'Foo';
1112
}
1213
}
1314

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);
15+
expect(instanceOf(undefined, true, Foo)).to.equal(false);
16+
expect(instanceOf(undefined, null, Foo)).to.equal(false);
17+
expect(instanceOf(undefined, Object.create(null), Foo)).to.equal(false);
1718
});
1819

1920
it('detect name clashes with older versions of this lib', () => {
@@ -24,6 +25,7 @@ describe('instanceOf', () => {
2425

2526
function newVersion() {
2627
class Foo {
28+
readonly __isFoo = true as const;
2729
get [Symbol.toStringTag]() {
2830
return 'Foo';
2931
}
@@ -33,13 +35,17 @@ describe('instanceOf', () => {
3335

3436
const NewClass = newVersion();
3537
const OldClass = oldVersion();
36-
expect(instanceOf(new NewClass(), NewClass)).to.equal(true);
37-
expect(() => instanceOf(new OldClass(), NewClass)).to.throw();
38+
const newInstance = new NewClass();
39+
expect(instanceOf(newInstance.__isFoo, newInstance, NewClass)).to.equal(
40+
true,
41+
);
42+
expect(() => instanceOf(undefined, new OldClass(), NewClass)).to.throw();
3843
});
3944

4045
it('allows instances to have share the same constructor name', () => {
4146
function getMinifiedClass(tag: string) {
4247
class SomeNameAfterMinification {
48+
readonly [tag] = true as const;
4349
get [Symbol.toStringTag]() {
4450
return tag;
4551
}
@@ -49,17 +55,25 @@ describe('instanceOf', () => {
4955

5056
const Foo = getMinifiedClass('Foo');
5157
const Bar = getMinifiedClass('Bar');
52-
expect(instanceOf(new Foo(), Bar)).to.equal(false);
53-
expect(instanceOf(new Bar(), Foo)).to.equal(false);
58+
const fooInstance = new Foo();
59+
const barInstance = new Bar();
60+
expect(instanceOf(fooInstance.foo, fooInstance, Bar)).to.equal(false);
61+
expect(instanceOf(barInstance.bar, barInstance, Foo)).to.equal(false);
5462

5563
const DuplicateOfFoo = getMinifiedClass('Foo');
56-
expect(() => instanceOf(new DuplicateOfFoo(), Foo)).to.throw();
57-
expect(() => instanceOf(new Foo(), DuplicateOfFoo)).to.throw();
64+
const duplicateOfFooInstance = new DuplicateOfFoo();
65+
expect(() =>
66+
instanceOf(duplicateOfFooInstance.foo, new DuplicateOfFoo(), Foo),
67+
).to.throw();
68+
expect(() =>
69+
instanceOf(fooInstance.foo, fooInstance, DuplicateOfFoo),
70+
).to.throw();
5871
});
5972

6073
it('fails with descriptive error message', () => {
6174
function getFoo() {
6275
class Foo {
76+
readonly __isFoo = true as const;
6377
get [Symbol.toStringTag]() {
6478
return 'Foo';
6579
}
@@ -69,11 +83,14 @@ describe('instanceOf', () => {
6983
const Foo1 = getFoo();
7084
const Foo2 = getFoo();
7185

72-
expect(() => instanceOf(new Foo1(), Foo2)).to.throw(
73-
/^Cannot use Foo "{}" from another module or realm./m,
86+
const foo1Instance = new Foo1();
87+
const foo2Instance = new Foo2();
88+
89+
expect(() => instanceOf(foo1Instance.__isFoo, foo1Instance, Foo2)).to.throw(
90+
/^Cannot use Foo "{ __isFoo: true }" from another module or realm./m,
7491
);
75-
expect(() => instanceOf(new Foo2(), Foo1)).to.throw(
76-
/^Cannot use Foo "{}" from another module or realm./m,
92+
expect(() => instanceOf(foo2Instance.__isFoo, foo2Instance, Foo1)).to.throw(
93+
/^Cannot use Foo "{ __isFoo: true }" from another module or realm./m,
7794
);
7895
});
7996
});

src/jsutils/instanceOf.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,30 @@ const isProduction =
77
process.env.NODE_ENV === 'production';
88

99
/**
10-
* A replacement for instanceof which includes an error warning when multi-realm
11-
* constructors are detected.
10+
* A utility which throws an error warning when multi-realm constructors are detected,
11+
* triggered by a positive brand check and a negative instanceof (for an object).
12+
*
13+
* When not in development mode, only the brand check is used.
14+
*
1215
* See: https://expressjs.com/en/advanced/best-practice-performance.html#set-node_env-to-production
1316
* See: https://webpack.js.org/guides/production/
1417
*/
15-
export const instanceOf: (value: unknown, constructor: Constructor) => boolean =
16-
/* c8 ignore next 6 */
18+
export const instanceOf: (
19+
brandCheck: true | undefined,
20+
value: unknown,
21+
constructor: Constructor,
22+
) => boolean =
23+
/* c8 ignore next 9 */
1724
// FIXME: https://github.com/graphql/graphql-js/issues/2317
1825
isProduction
19-
? function instanceOf(value: unknown, constructor: Constructor): boolean {
20-
return value instanceof constructor;
26+
? function instanceOf(brandCheck: true | undefined): boolean {
27+
return brandCheck === true;
2128
}
22-
: function instanceOf(value: unknown, constructor: Constructor): boolean {
29+
: function instanceOf(
30+
_brandCheck: true | undefined,
31+
value: unknown,
32+
constructor: Constructor,
33+
): boolean {
2334
if (value instanceof constructor) {
2435
return true;
2536
}

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)