Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new primitive PropType Symbol #6377

Merged
merged 14 commits into from
May 10, 2016
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? Doesn’t appear to be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a leftover from a @puradox fix on tests — I will clean up this!

On Sat, Apr 9, 2016, 00:26 Dan Abramov notifications@github.com wrote:

In package.json
#6377 (comment):

 "coveralls": "^2.11.6",
 "del": "^2.0.2",
 "derequire": "^2.0.3",
  • "es6-symbol": "^3.0.2",

Is this necessary? Doesn’t appear to be used.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/facebook/react/pull/6377/files/ad94295f139a8190286852e14a5c731065f5f0ba#r59097655

Ryan Lahfa, de mon téléphone.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @RaitoBezarius! I forgot to remove this dependency when removing unit test support for es6-symbol.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have pushed a fix on your fork to remove the dependency, so that we can
unblock the code review.

Kind regards, Ryan Lahfa

"eslint": "1.10.3",
"eslint-plugin-react": "4.1.0",
"eslint-plugin-react-internal": "file:eslint-rules",
Expand Down
23 changes: 23 additions & 0 deletions src/isomorphic/classic/types/ReactPropTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ var ReactPropTypes = {
number: createPrimitiveTypeChecker('number'),
object: createPrimitiveTypeChecker('object'),
string: createPrimitiveTypeChecker('string'),
symbol: createPrimitiveTypeChecker('symbol'),

any: createAnyTypeChecker(),
arrayOf: createArrayOfTypeChecker,
Expand Down Expand Up @@ -406,6 +407,25 @@ function isNode(propValue) {
}
}

function isSymbol(propType, propValue) {
// Native Symbol.
if (propType === 'symbol') {
return true;
}

// 19.4.3.5 Symbol.prototype[@@toStringTag] === 'Symbol'
if (propValue['@@toStringTag'] === 'Symbol') {
return true;
}

// Fallback for non-spec compliant Symbols which are polyfilled.
if (typeof Symbol === 'function' && propValue instanceof Symbol) {
return true;
}

return false;
}

// Equivalent of `typeof` but with special handling for array and regexp.
function getPropType(propValue) {
var propType = typeof propValue;
Expand All @@ -418,6 +438,9 @@ function getPropType(propValue) {
// passes PropTypes.object.
return 'object';
}
if (isSymbol(propType, propValue)) {
return 'symbol';
}
return propType;
}

Expand Down
49 changes: 49 additions & 0 deletions src/isomorphic/classic/types/__tests__/ReactPropTypes-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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() {
Expand All @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -779,6 +798,36 @@ 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 CoreSymbol = require('core-js/library/es6/symbol');
typeCheckPass(PropTypes.symbol, CoreSymbol('core-js'));
});
});

describe('Custom validator', function() {
beforeEach(function() {
jest.resetModuleRegistry();
Expand Down