From 97bcce5214f94ceaf525161dba27db6ee18828c4 Mon Sep 17 00:00:00 2001 From: Sam Balana Date: Tue, 29 Mar 2016 17:18:34 -0700 Subject: [PATCH 01/11] Added Symbol primative PropType. With the edition of ECMA-262, we now have a new primative type called Symbol. This primative type should be added to the PropTypes, as users will eventually be using Symbols in their props for describing unique and immutable data, such as identifiers. --- src/isomorphic/classic/types/ReactPropTypes.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/isomorphic/classic/types/ReactPropTypes.js b/src/isomorphic/classic/types/ReactPropTypes.js index 99c036496aaba..f0926fd4d75eb 100644 --- a/src/isomorphic/classic/types/ReactPropTypes.js +++ b/src/isomorphic/classic/types/ReactPropTypes.js @@ -73,6 +73,7 @@ var ReactPropTypes = { number: createPrimitiveTypeChecker('number'), object: createPrimitiveTypeChecker('object'), string: createPrimitiveTypeChecker('string'), + symbol: createPrimitiveTypeChecker('symbol'), any: createAnyTypeChecker(), arrayOf: createArrayOfTypeChecker, From 75c4e037183ec99d6e961d3710cfb0f19b0415dd Mon Sep 17 00:00:00 2001 From: Sam Balana Date: Tue, 29 Mar 2016 17:20:50 -0700 Subject: [PATCH 02/11] Added tests for the new primative, Symbol. I believe that I covered all the typical use cases for Symbols. If you think that I missed one, feel free to contribute. --- .../types/__tests__/ReactPropTypes-test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index abfa976b00fe3..bcc5bce2a5874 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -80,6 +80,12 @@ describe('ReactPropTypes', function() { 'Invalid prop `testProp` of type `object` supplied to ' + '`testComponent`, expected `string`.' ); + typeCheckFail( + PropTypes.string, + Symbol(), + 'Invalid prop `testProp` of type `symbol` supplied to ' + + '`testComponent`, expected `string`.' + ); }); it('should fail date and regexp correctly', function() { @@ -106,6 +112,7 @@ describe('ReactPropTypes', function() { typeCheckPass(PropTypes.object, {}); typeCheckPass(PropTypes.object, new Date()); typeCheckPass(PropTypes.object, /please/); + typeCheckPass(PropTypes.symbol, Symbol()); }); it('should be implicitly optional and not warn without values', function() { @@ -124,6 +131,7 @@ describe('ReactPropTypes', function() { typeCheckPass(PropTypes.any, 0); typeCheckPass(PropTypes.any, 'str'); typeCheckPass(PropTypes.any, []); + typeCheckPass(PropTypes.any, Symbol()); }); it('should be implicitly optional and not warn without values', function() { @@ -150,6 +158,7 @@ describe('ReactPropTypes', function() { typeCheckPass(PropTypes.arrayOf(PropTypes.number), [1, 2, 3]); typeCheckPass(PropTypes.arrayOf(PropTypes.string), ['a', 'b', 'c']); typeCheckPass(PropTypes.arrayOf(PropTypes.oneOf(['a', 'b'])), ['a', 'b']); + typeCheckPass(PropTypes.arrayOf(PropTypes.symbol), [Symbol(), Symbol()]); }); it('should support arrayOf with complex types', function() { @@ -487,6 +496,10 @@ describe('ReactPropTypes', function() { PropTypes.objectOf(PropTypes.oneOf(['a', 'b'])), {a: 'a', b: 'b'} ); + typeCheckPass( + PropTypes.objectOf(PropTypes.symbol), + {a: Symbol(), b: Symbol(), c: Symbol()} + ); }); it('should support objectOf with complex types', function() { @@ -542,6 +555,12 @@ describe('ReactPropTypes', function() { 'Invalid prop `testProp` of type `string` supplied to ' + '`testComponent`, expected an object.' ); + typeCheckFail( + PropTypes.objectOf(PropTypes.symbol), + Symbol(), + 'Invalid prop `testProp` of type `symbol` supplied to ' + + '`testComponent`, expected an object.' + ); }); it('should not warn when passing an empty object', function() { From 5485ac8f6c77a487d7627bf2b994992e9e03c1e3 Mon Sep 17 00:00:00 2001 From: Sam Balana Date: Thu, 31 Mar 2016 13:47:03 -0700 Subject: [PATCH 03/11] Support for ES6 polyfills Most ES6 polyfills will add support by implementing `Symbol` as a function. This causes `typeof` to return `function` rather than `symbol` for polyfilled clients. --- src/isomorphic/classic/types/ReactPropTypes.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/isomorphic/classic/types/ReactPropTypes.js b/src/isomorphic/classic/types/ReactPropTypes.js index f0926fd4d75eb..0f06af31a68b4 100644 --- a/src/isomorphic/classic/types/ReactPropTypes.js +++ b/src/isomorphic/classic/types/ReactPropTypes.js @@ -419,6 +419,12 @@ function getPropType(propValue) { // passes PropTypes.object. return 'object'; } + if (propType === 'function' && typeof Symbol === 'function' && + propValue instanceof Symbol) { + // ES6 polyfills will return 'function' rather than 'symbol' for typeof a + // Symbol. + return 'symbol'; + } return propType; } From 13d9720804a828666d57994d5a40dc5e66ec12d0 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 2 Apr 2016 10:10:05 +0200 Subject: [PATCH 04/11] prop-types(symbols): add a helper is-symbol function --- .../classic/types/ReactPropTypes.js | 28 ++++++++++++++++--- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/isomorphic/classic/types/ReactPropTypes.js b/src/isomorphic/classic/types/ReactPropTypes.js index 0f06af31a68b4..92ef25d337f04 100644 --- a/src/isomorphic/classic/types/ReactPropTypes.js +++ b/src/isomorphic/classic/types/ReactPropTypes.js @@ -407,6 +407,29 @@ function isNode(propValue) { } } +function isSymbol(propType, propValue) { + if (propType === 'symbol') { + return true; // This is a native Symbol. + } + + if (typeof Symbol === 'undefined') { + // No Symbol is available in the global namespace. + // We need to check if it has some spec-defined method (duck typing). + + if (propValue['@@toStringTag'] === 'Symbol') { + return true; + } + + if (propValue.__key__ && propValue.prototype.__key__ === undefined) { + return propValue.toString() === 'Symbol' + } + } else { + return propValue instanceof Symbol; // This is a polyfilled Symbol. + } + + return false; +} + // Equivalent of `typeof` but with special handling for array and regexp. function getPropType(propValue) { var propType = typeof propValue; @@ -419,10 +442,7 @@ function getPropType(propValue) { // passes PropTypes.object. return 'object'; } - if (propType === 'function' && typeof Symbol === 'function' && - propValue instanceof Symbol) { - // ES6 polyfills will return 'function' rather than 'symbol' for typeof a - // Symbol. + if (isSymbol(propType, propValue)) { return 'symbol'; } return propType; From 0d8ab0efa3520a45754e22353d97e671c16bf9e4 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 2 Apr 2016 14:26:35 +0200 Subject: [PATCH 05/11] devDependencies: add core-js and es6-symbol polyfill for tests --- package.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/package.json b/package.json index b2d8774aab3c5..2e478eb1fcd67 100644 --- a/package.json +++ b/package.json @@ -29,9 +29,11 @@ "browserify": "^12.0.1", "bundle-collapser": "^1.1.1", "coffee-script": "^1.8.0", + "core-js": "^2.2.1", "coveralls": "^2.11.6", "del": "^2.0.2", "derequire": "^2.0.3", + "es6-symbol": "^3.0.2", "eslint": "1.10.3", "eslint-plugin-react": "4.1.0", "eslint-plugin-react-internal": "file:eslint-rules", From 4239f8ac3bf3dcbcc8554e3b821f562831eeb663 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 2 Apr 2016 14:27:02 +0200 Subject: [PATCH 06/11] prop-types(symbols): rewrite isSymbol to be simpler 1) If it is a native Symbol 2) If it is spec-compliant 3) Try to match non-spec compliant polyfill by a instanceof check on Symbol if it exists in the global namespace --- .../classic/types/ReactPropTypes.js | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/isomorphic/classic/types/ReactPropTypes.js b/src/isomorphic/classic/types/ReactPropTypes.js index 92ef25d337f04..aa15eb6567cea 100644 --- a/src/isomorphic/classic/types/ReactPropTypes.js +++ b/src/isomorphic/classic/types/ReactPropTypes.js @@ -408,23 +408,19 @@ function isNode(propValue) { } function isSymbol(propType, propValue) { + // Native Symbol. if (propType === 'symbol') { - return true; // This is a native Symbol. + return true; } - if (typeof Symbol === 'undefined') { - // No Symbol is available in the global namespace. - // We need to check if it has some spec-defined method (duck typing). - - if (propValue['@@toStringTag'] === 'Symbol') { - return true; - } + // 19.4.3.5 Symbol.prototype[@@toStringTag] === 'Symbol' + if (propValue['@@toStringTag'] === 'Symbol') { + return true; + } - if (propValue.__key__ && propValue.prototype.__key__ === undefined) { - return propValue.toString() === 'Symbol' - } - } else { - return propValue instanceof Symbol; // This is a polyfilled Symbol. + // Fallback for non-spec compliant Symbols which are polyfilled. + if (typeof Symbol !== 'undefined' && propValue instanceof Symbol) { + return true; } return false; From c291f8a81e83b6bb9b7322cc9fdff83954c8e6e2 Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Sat, 2 Apr 2016 14:28:08 +0200 Subject: [PATCH 07/11] prop-types(symbols,tests): test for polyfilled Symbol and non-Symbol elements --- .../types/__tests__/ReactPropTypes-test.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index bcc5bce2a5874..eea4b7577a740 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -798,6 +798,39 @@ describe('ReactPropTypes', function() { }); }); + describe('Symbol Type', function() { + it('should warn for non-symbol', function() { + typeCheckFail( + PropTypes.symbol, + 'hello', + 'Invalid prop `testProp` of type `string` supplied to ' + + '`testComponent`, expected `symbol`.' + ); + typeCheckFail( + PropTypes.symbol, + function () { }, + 'Invalid prop `testProp` of type `function` supplied to ' + + '`testComponent`, expected `symbol`.' + ); + typeCheckFail( + PropTypes.symbol, + { + '@@toStringTag': 'Katana' + }, + 'Invalid prop `testProp` of type `object` supplied to ' + + '`testComponent`, expected `symbol`.' + ); + }); + + it('should not warn for a polyfilled Symbol', function() { + var ES6Symbol = require('es6-symbol/polyfill'); + var CoreSymbol = require('core-js/library/es6/symbol'); + + typeCheckPass(PropTypes.symbol, ES6Symbol('es6-symbol')) + typeCheckPass(PropTypes.symbol, CoreSymbol('core-js')) + }); + }); + describe('Custom validator', function() { beforeEach(function() { jest.resetModuleRegistry(); From a98e2cb8b156abb7298df6ef2c3d46564f22ef7f Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Tue, 5 Apr 2016 22:01:37 +0200 Subject: [PATCH 08/11] prop-types(tests,code-style): fix missing semicolon, unexpected spaces and trailing comma --- .../classic/types/__tests__/ReactPropTypes-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index eea4b7577a740..bfd6f8a410cba 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -808,14 +808,14 @@ describe('ReactPropTypes', function() { ); typeCheckFail( PropTypes.symbol, - function () { }, + function() { }, 'Invalid prop `testProp` of type `function` supplied to ' + '`testComponent`, expected `symbol`.' ); typeCheckFail( PropTypes.symbol, { - '@@toStringTag': 'Katana' + '@@toStringTag': 'Katana', }, 'Invalid prop `testProp` of type `object` supplied to ' + '`testComponent`, expected `symbol`.' @@ -826,8 +826,8 @@ describe('ReactPropTypes', function() { var ES6Symbol = require('es6-symbol/polyfill'); var CoreSymbol = require('core-js/library/es6/symbol'); - typeCheckPass(PropTypes.symbol, ES6Symbol('es6-symbol')) - typeCheckPass(PropTypes.symbol, CoreSymbol('core-js')) + typeCheckPass(PropTypes.symbol, ES6Symbol('es6-symbol')); + typeCheckPass(PropTypes.symbol, CoreSymbol('core-js')); }); }); From 05820f89c15159994247899721aef8ae14c7ead9 Mon Sep 17 00:00:00 2001 From: Sam Balana Date: Thu, 7 Apr 2016 16:57:11 -0700 Subject: [PATCH 09/11] More specific typeof check for polyfilled Symbol Let's get more specific with our tests. Symbol has to be a function if it's a polyfill. There are no object polyfills out there. (If there is, ping me and I'll add support) Instead of checking if Symbol is undefined, let's check if it's a function instead since instanceof is depending that Symbol is already a function. --- src/isomorphic/classic/types/ReactPropTypes.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/isomorphic/classic/types/ReactPropTypes.js b/src/isomorphic/classic/types/ReactPropTypes.js index aa15eb6567cea..4d9ac1d846107 100644 --- a/src/isomorphic/classic/types/ReactPropTypes.js +++ b/src/isomorphic/classic/types/ReactPropTypes.js @@ -419,7 +419,7 @@ function isSymbol(propType, propValue) { } // Fallback for non-spec compliant Symbols which are polyfilled. - if (typeof Symbol !== 'undefined' && propValue instanceof Symbol) { + if (typeof Symbol === 'function' && propValue instanceof Symbol) { return true; } From ad94295f139a8190286852e14a5c731065f5f0ba Mon Sep 17 00:00:00 2001 From: Sam Balana Date: Thu, 7 Apr 2016 17:41:42 -0700 Subject: [PATCH 10/11] Removed test support for medikoo/es6-symbol There seems to be a bug with medikoo/es6-symbol where the global state affects the implementation of the polyfill. I found this by running the individual test file alone then running all the tests using `grunt test`. I found that it passes when ran alone and failed when ran collectively. I did not find this buggy behavior with zloirock/core-js's implementation of a polyfill for Symbol. Thus, removing I will keep the more popular polyfill (core-js) and remove the buggy polyfill (es6-symbol). If you are reading this and can think of a confounding factor that is causing this bug, please let me know and we can try to work together to add support for medikoo/es6-symbol. --- src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js index bfd6f8a410cba..68c233ceb9d11 100644 --- a/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js +++ b/src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js @@ -823,10 +823,7 @@ describe('ReactPropTypes', function() { }); it('should not warn for a polyfilled Symbol', function() { - var ES6Symbol = require('es6-symbol/polyfill'); var CoreSymbol = require('core-js/library/es6/symbol'); - - typeCheckPass(PropTypes.symbol, ES6Symbol('es6-symbol')); typeCheckPass(PropTypes.symbol, CoreSymbol('core-js')); }); }); From 95fed016342e7e917891eb3e9e9bff717169a7ed Mon Sep 17 00:00:00 2001 From: Raito Bezarius Date: Mon, 11 Apr 2016 20:19:09 +0200 Subject: [PATCH 11/11] package(dependency): remove unused es6-symbol dependency --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 2e478eb1fcd67..b8dbc42f2f65b 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,6 @@ "coveralls": "^2.11.6", "del": "^2.0.2", "derequire": "^2.0.3", - "es6-symbol": "^3.0.2", "eslint": "1.10.3", "eslint-plugin-react": "4.1.0", "eslint-plugin-react-internal": "file:eslint-rules",