Skip to content

Commit 1484d50

Browse files
author
Brielle Harrison
committed
Addressing feedback and general cleanup
**Changes** * Remove `Symbol.toStringTag` from the classes themselves, leaving only the prototype application * Refactor the injection of the symbols to a reusable utility in jsUtils. * Write new tests for the utility functions * Update tests for the classes receiving Symbol.toStringTag
1 parent dfaf443 commit 1484d50

File tree

8 files changed

+5143
-241
lines changed

8 files changed

+5143
-241
lines changed

package-lock.json

Lines changed: 4888 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* Copyright (c) 2015-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
import { expect } from 'chai';
9+
import { describe, it } from 'mocha';
10+
import { hasSymbolSupport, applyToStringTag } from '../symbolSupport';
11+
12+
describe('symbolSupportTests', () => {
13+
// NOTE Symbol appeared in nodejs in 0.12.18 but was largely unusable in
14+
// that format. Symbol.toStringTag showed up in node 6.4.0, but was available
15+
// behind a flag as early as 4.9.1.
16+
const [, major, minor, patch] = /^v(\d+)\.?(\d+)?\.?(\d+)?/.exec(
17+
process.version,
18+
);
19+
20+
it('should have Symbol in scope if the version >= 0.12.18', () => {
21+
expect(major >= 4).to.equal(true);
22+
23+
if (major === 4) {
24+
expect(minor >= 9).to.equal(true);
25+
}
26+
27+
if (minor === 9) {
28+
expect(patch >= 1).to.equal(true);
29+
}
30+
31+
expect(hasSymbolSupport()).to.equal(true);
32+
});
33+
34+
it('should have Symbol in scope if the version >= 0.12.18', () => {
35+
expect(major >= 6).to.equal(true);
36+
37+
if (major === 6) {
38+
expect(minor >= 4).to.equal(true);
39+
}
40+
41+
if (minor === 4) {
42+
expect(patch >= 0).to.equal(true);
43+
}
44+
45+
expect(typeof Symbol !== 'undefined').to.equal(true);
46+
expect(hasSymbolSupport('toStringTag')).to.equal(true);
47+
});
48+
49+
it('should be able to apply toStringTag to a class', () => {
50+
class A {}
51+
applyToStringTag(A);
52+
53+
const a = new A();
54+
55+
expect(Object.prototype.toString.call(a)).to.equal('[object A]');
56+
expect(Object.prototype.toString.call(A)).not.to.equal('[object A]');
57+
});
58+
});

src/jsutils/symbolSupport.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
/**
2+
* Copyright (c) 2015-present, Facebook, Inc.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict
8+
*/
9+
10+
/**
11+
* A function that can either simply determine if `Symbol`s are allowed as
12+
* well as, optionally, determine whether or not a property of symbol
13+
* exists.
14+
*
15+
* If the name of a specific `Symbol` property is supplied, the resulting
16+
* value will only be true if property is mapped to an instance of `Symbol`
17+
* such as `.toStringTag`, `.iterator`, `.species`, `.isConcatSpreadable`,
18+
* etc...
19+
*
20+
* @method hasSymbolSupport
21+
*
22+
* @param {string} specificSymbol an optional name of a property on the
23+
* `Symbol` class itself that statically refers to a predefined `Symbol`. If
24+
* properly specified and the value is set, then true will be returned.
25+
* @return {bool} true if `Symbol` is both defined and a function. Optionally
26+
* true if the `Symbol` class is true, a predefined symbol name such as
27+
* `toStringTag` is set on the `Symbol` class and it maps to an instance of
28+
* `Symbol`. False in all other cases
29+
*/
30+
export function hasSymbolSupport(specificSymbol?: string): boolean {
31+
const hasSymbols: boolean = typeof Symbol === 'function';
32+
33+
if (!hasSymbols) {
34+
return false;
35+
}
36+
37+
if (specificSymbol) {
38+
// NOTE: The capture of type and string comparison over a few lines is
39+
// necessary to appease the lint and flowtype gods.
40+
//
41+
// ((typeof Symbol[specificSymbol]): string) !== 'symbol' makes lint angry
42+
// and typeof Symbol[specificSymbol] !== 'symbol' makes flow angry
43+
//
44+
// The former thinks everything is too verbose the later thinks I am
45+
// comparing Symbol instance rather than the string resulting from a call
46+
// to typeof.
47+
//
48+
// Le sigh....
49+
const type: string = typeof Symbol[specificSymbol];
50+
51+
if (type !== 'symbol') {
52+
return false;
53+
}
54+
}
55+
56+
return true;
57+
}
58+
59+
/**
60+
* The `applyToStringTag()` function checks first to see if the runtime
61+
* supports the `Symbol` class and then if the `Symbol.toStringTag` constant
62+
* is defined as a `Symbol` instance. If both conditions are met, the
63+
* Symbol.toStringTag property is defined as a getter that returns the
64+
* supplied class constructor's name.
65+
*
66+
* @method applyToStringTag
67+
*
68+
* @param {Class<*>} classObject a class such as Object, String, Number but
69+
* typically one of your own creation through the class keyword; `class A {}`,
70+
* for example.
71+
*/
72+
export function applyToStringTag(classObject: Class<*>): void {
73+
if (hasSymbolSupport('toStringTag')) {
74+
Object.defineProperty(classObject.prototype, Symbol.toStringTag, {
75+
get() {
76+
return this.constructor.name;
77+
},
78+
});
79+
}
80+
}

src/language/source.js

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
*/
99

1010
import invariant from '../jsutils/invariant';
11+
import { applyToStringTag } from '../jsutils/symbolSupport';
1112

1213
type Location = {
1314
line: number,
@@ -42,14 +43,5 @@ export class Source {
4243
}
4344
}
4445

45-
Object.defineProperty(Source, Symbol.toStringTag, {
46-
get() {
47-
return this.name;
48-
},
49-
});
50-
51-
Object.defineProperty(Source.prototype, Symbol.toStringTag, {
52-
get() {
53-
return this.constructor.name;
54-
},
55-
});
46+
// Conditionally apply `[Symbol.toStringTag]` if `Symbol`s are supported
47+
applyToStringTag(Source);

0 commit comments

Comments
 (0)