From 685dee7cb167200572ba0f22cb0c34593cbf0cd7 Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Thu, 6 Apr 2017 17:43:33 -0600 Subject: [PATCH 1/8] Initial work for #348 --- src/index.js | 39 +++++++++++++++++++++++---------------- test/component.js | 22 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/index.js b/src/index.js index aa9c2da..78bc0a6 100644 --- a/src/index.js +++ b/src/index.js @@ -293,7 +293,7 @@ function normalizeVNode(vnode) { } applyEventNormalization(vnode); - + validatePropTypes(vnode); return vnode; } @@ -318,6 +318,28 @@ function cloneElement(element, props, ...children) { return normalizeVNode(preactCloneElement(...cloneArgs)); } +function validatePropTypes (vnode) { + let componentClass = typeof vnode.nodeName === 'function' ? vnode.nodeName : vnode.type; + + if (typeof componentClass !== 'function') return; + let name = ( + componentClass.prototype.displayName + || componentClass.displayName + || componentClass.name + ); + + let propTypes = componentClass.propTypes; + + if (propTypes) { + let props = vnode.props; + for (let propKey in propTypes) { + if (propTypes.hasOwnProperty(propKey) && typeof propTypes[propKey] === 'function') { + let err = propTypes[propKey](props, propKey, name, 'prop'); + if (err) console.error(new Error(err.message || err)); + } + } + } +} function isValidElement(element) { return element && ((element instanceof VNode) || element.$$typeof===REACT_ELEMENT_TYPE); @@ -509,21 +531,6 @@ function propsHook(props, context) { props.children[0] = props.children; } } - - // add proptype checking - if (DEV) { - let ctor = typeof this==='function' ? this : this.constructor, - propTypes = this.propTypes || ctor.propTypes; - if (propTypes) { - for (let prop in propTypes) { - if (propTypes.hasOwnProperty(prop) && typeof propTypes[prop]==='function') { - const displayName = this.displayName || ctor.name; - let err = propTypes[prop](props, prop, displayName, 'prop'); - if (err) console.error(new Error(err.message || err)); - } - } - } - } } diff --git a/test/component.js b/test/component.js index cb2a877..22537d4 100644 --- a/test/component.js +++ b/test/component.js @@ -236,6 +236,28 @@ describe('components', () => { }; checkPropTypes(Foo2); }); + + it.only('should handle function as children propTypes', () => { + function Foo (props) { return React.Children.only(props.children)(); } + + Foo.propTypes = { + children: React.PropTypes.func.isRequired + }; + + sinon.stub(console, 'error'); + + React.render(, scratch); + expect(console.error).to.have.been.calledWithMatch({ + message: 'Children.only() expects only one child.' + }); + + console.error.reset(); + + React.render({() =>
}, scratch); + expect(console.error).not.to.have.been.called; + + console.error.restore(); + }); }); describe("mixins", () => { From bd4c38a7c4ae52aef7ff3e8e1db365aef91b40f1 Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Fri, 7 Apr 2017 00:06:30 -0600 Subject: [PATCH 2/8] Only run propType checks during createElement or cloneElement - moved the validation call into createElement/cloneElement to match React - added test for cloneElement #348 --- src/index.js | 68 ++++++++++++++++++++++++++++------------------- test/component.js | 30 ++++++--------------- 2 files changed, 48 insertions(+), 50 deletions(-) diff --git a/src/index.js b/src/index.js index 78bc0a6..f8b525e 100644 --- a/src/index.js +++ b/src/index.js @@ -270,10 +270,45 @@ function statelessComponentHook(Ctor) { return Wrapped; } +function validatePropTypes (vnode) { + if (DEV) { + let componentClass = typeof vnode.nodeName === "function" + ? vnode.nodeName + : vnode.type; + + if (typeof componentClass !== "function") return; + let name = componentClass.displayName || componentClass.name; + let propTypes = componentClass.propTypes; + if (propTypes) { + let props = vnode.props.children + ? vnode.props + : { + ...vnode.props, + ...{ + children: Array.isArray(vnode.children) && + vnode.children.length === 1 + ? vnode.children[0] + : vnode.children + } + }; + for (let propKey in propTypes) { + if ( + propTypes.hasOwnProperty(propKey) && + typeof propTypes[propKey] === "function" + ) { + let err = propTypes[propKey](props, propKey, name, "prop"); + if (err) console.error(new Error(err.message || err)); + } + } + } + } +} function createElement(...args) { upgradeToVNodes(args, 2); - return normalizeVNode(h(...args)); + let node = h(...args); + validatePropTypes(node); + return normalizeVNode(node); } @@ -293,7 +328,6 @@ function normalizeVNode(vnode) { } applyEventNormalization(vnode); - validatePropTypes(vnode); return vnode; } @@ -315,30 +349,9 @@ function cloneElement(element, props, ...children) { else if (props && props.children) { cloneArgs.push(props.children); } - return normalizeVNode(preactCloneElement(...cloneArgs)); -} - -function validatePropTypes (vnode) { - let componentClass = typeof vnode.nodeName === 'function' ? vnode.nodeName : vnode.type; - - if (typeof componentClass !== 'function') return; - let name = ( - componentClass.prototype.displayName - || componentClass.displayName - || componentClass.name - ); - - let propTypes = componentClass.propTypes; - - if (propTypes) { - let props = vnode.props; - for (let propKey in propTypes) { - if (propTypes.hasOwnProperty(propKey) && typeof propTypes[propKey] === 'function') { - let err = propTypes[propKey](props, propKey, name, 'prop'); - if (err) console.error(new Error(err.message || err)); - } - } - } + let newNode = preactCloneElement(...cloneArgs); + validatePropTypes(newNode); + return normalizeVNode(newNode); } function isValidElement(element) { @@ -511,7 +524,7 @@ function multihook(hooks, skipDuplicates) { function newComponentHook(props, context) { - propsHook.call(this, props, context); + propsHook(props, context); this.componentWillReceiveProps = multihook([propsHook, this.componentWillReceiveProps || 'componentWillReceiveProps']); this.render = multihook([propsHook, beforeRender, this.render || 'render', afterRender]); } @@ -533,7 +546,6 @@ function propsHook(props, context) { } } - function beforeRender(props) { currentComponent = this; } diff --git a/test/component.js b/test/component.js index 22537d4..a4ec257 100644 --- a/test/component.js +++ b/test/component.js @@ -191,6 +191,14 @@ describe('components', () => { message: 'Invalid prop `bool` of type `string` supplied to `Foo`, expected `boolean`.' }); + console.error.reset(); + + const clone = React.cloneElement( {}} bool="one"/>); + React.render(clone, scratch); + expect(console.error).to.have.been.calledWithMatch({ + message: 'Invalid prop `bool` of type `string` supplied to `Foo`, expected `boolean`.' + }); + console.error.restore(); } @@ -236,28 +244,6 @@ describe('components', () => { }; checkPropTypes(Foo2); }); - - it.only('should handle function as children propTypes', () => { - function Foo (props) { return React.Children.only(props.children)(); } - - Foo.propTypes = { - children: React.PropTypes.func.isRequired - }; - - sinon.stub(console, 'error'); - - React.render(, scratch); - expect(console.error).to.have.been.calledWithMatch({ - message: 'Children.only() expects only one child.' - }); - - console.error.reset(); - - React.render({() =>
}, scratch); - expect(console.error).not.to.have.been.called; - - console.error.restore(); - }); }); describe("mixins", () => { From 5a337b009360352269c68196e872e7c86bcefd66 Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Fri, 7 Apr 2017 12:08:47 -0600 Subject: [PATCH 3/8] Normalize children how React wants them. #348 --- src/index.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/index.js b/src/index.js index f8b525e..7aa0873 100644 --- a/src/index.js +++ b/src/index.js @@ -280,17 +280,15 @@ function validatePropTypes (vnode) { let name = componentClass.displayName || componentClass.name; let propTypes = componentClass.propTypes; if (propTypes) { - let props = vnode.props.children - ? vnode.props - : { - ...vnode.props, - ...{ - children: Array.isArray(vnode.children) && - vnode.children.length === 1 - ? vnode.children[0] - : vnode.children - } - }; + let props = vnode.props; + if ( + !(vnode.props && vnode.props.children) && + vnode.children && + vnode.children.length + ) { + props.children = vnode.children; + } + propsHook(props); for (let propKey in propTypes) { if ( propTypes.hasOwnProperty(propKey) && From baf17862d61d12cc27b73fa98aed581bc905d0ac Mon Sep 17 00:00:00 2001 From: Neil Duffy Date: Fri, 14 Apr 2017 20:52:39 -0500 Subject: [PATCH 4/8] using react's prop-types lib (#359) --- package.json | 2 +- rollup.config.js | 2 +- src/index.js | 2 +- test/component.js | 34 +++++++++++++++++----------------- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 7a1f67b..8092d7b 100644 --- a/package.json +++ b/package.json @@ -82,7 +82,7 @@ "immutability-helper": "^2.1.2", "preact-render-to-string": "^3.6.0", "preact-transition-group": "^1.1.0", - "proptypes": "^0.14.3", + "prop-types": "^15.5.8", "standalone-react-addons-pure-render-mixin": "^0.1.1" } } diff --git a/rollup.config.js b/rollup.config.js index 9221e3d..d14decd 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -21,7 +21,7 @@ export default { useStrict: false, globals: { 'preact': 'preact', - 'proptypes': 'PropTypes' + 'prop-types': 'PropTypes' }, plugins: [ format==='umd' && memory({ diff --git a/src/index.js b/src/index.js index 7aa0873..f953bc6 100644 --- a/src/index.js +++ b/src/index.js @@ -1,4 +1,4 @@ -import PropTypes from 'proptypes'; +import PropTypes from 'prop-types'; import { render as preactRender, cloneElement as preactCloneElement, h, Component as PreactComponent, options } from 'preact'; const version = '15.1.0'; // trick libraries to think we are react diff --git a/test/component.js b/test/component.js index a4ec257..e48af9a 100644 --- a/test/component.js +++ b/test/component.js @@ -173,13 +173,13 @@ describe('components', () => { }); describe('propTypes', () => { - function checkPropTypes(Foo) { + function checkPropTypes(Foo, name = 'Foo') { sinon.stub(console, 'error'); - React.render(, scratch); - expect(console.error).to.have.been.calledWithMatch({ - message: 'Required prop `func` was not specified in `Foo`.' - }); + expect(console.error).to.have.been.calledWithMatch( + 'Warning: Failed prop type: The prop `func` is marked as required in `' + name + '`, but its value is `undefined`.' + ); + expect(console.error).to.have.been.called; console.error.reset(); @@ -187,9 +187,9 @@ describe('components', () => { expect(console.error).not.to.have.been.called; React.render({}} bool="one" />, scratch); - expect(console.error).to.have.been.calledWithMatch({ - message: 'Invalid prop `bool` of type `string` supplied to `Foo`, expected `boolean`.' - }); + expect(console.error).to.have.been.calledWithMatch( + 'Warning: Failed prop type: Invalid prop `bool` of type `string` supplied to `' + name + '`, expected `boolean`.' + ); console.error.reset(); @@ -217,7 +217,7 @@ describe('components', () => { }); it('should support propTypes for createClass components', () => { - const Foo = React.createClass({ + const Bar = React.createClass({ propTypes: { func: React.PropTypes.func.isRequired, bool: React.PropTypes.bool @@ -225,24 +225,24 @@ describe('components', () => { render: () =>
}); - checkPropTypes(Foo); + checkPropTypes(Bar, 'Bar'); }); it('should support propTypes for pure components', () => { - function Foo() { return
; } - Foo.propTypes = { + function Baz() { return
; } + Baz.propTypes = { func: React.PropTypes.func.isRequired, bool: React.PropTypes.bool }; - checkPropTypes(Foo); + checkPropTypes(Baz, 'Baz'); - const Foo2 = () =>
; - Foo2.displayName = 'Foo'; - Foo2.propTypes = { + const Bip = () =>
; + Bip.displayName = 'Bip'; + Bip.propTypes = { func: React.PropTypes.func.isRequired, bool: React.PropTypes.bool }; - checkPropTypes(Foo2); + checkPropTypes(Bip, 'Bip'); }); }); From 9aaba68e7db6ef2e98526c1783138462312e13d0 Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Thu, 6 Apr 2017 17:43:33 -0600 Subject: [PATCH 5/8] Initial work for #348 --- test/component.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/component.js b/test/component.js index e48af9a..b259388 100644 --- a/test/component.js +++ b/test/component.js @@ -244,6 +244,28 @@ describe('components', () => { }; checkPropTypes(Bip, 'Bip'); }); + + it.only('should handle function as children propTypes', () => { + function Foo (props) { return React.Children.only(props.children)(); } + + Foo.propTypes = { + children: React.PropTypes.func.isRequired + }; + + sinon.stub(console, 'error'); + + React.render(, scratch); + expect(console.error).to.have.been.calledWithMatch({ + message: 'Children.only() expects only one child.' + }); + + console.error.reset(); + + React.render({() =>
}, scratch); + expect(console.error).not.to.have.been.called; + + console.error.restore(); + }); }); describe("mixins", () => { From 4d44b487f857def3889852a8d3e7ecf59540bd48 Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Fri, 7 Apr 2017 00:06:30 -0600 Subject: [PATCH 6/8] Only run propType checks during createElement or cloneElement - moved the validation call into createElement/cloneElement to match React - added test for cloneElement #348 --- src/index.js | 31 +++++++++++++------------------ test/component.js | 22 ---------------------- 2 files changed, 13 insertions(+), 40 deletions(-) diff --git a/src/index.js b/src/index.js index f953bc6..d93a1ae 100644 --- a/src/index.js +++ b/src/index.js @@ -280,24 +280,19 @@ function validatePropTypes (vnode) { let name = componentClass.displayName || componentClass.name; let propTypes = componentClass.propTypes; if (propTypes) { - let props = vnode.props; - if ( - !(vnode.props && vnode.props.children) && - vnode.children && - vnode.children.length - ) { - props.children = vnode.children; - } - propsHook(props); - for (let propKey in propTypes) { - if ( - propTypes.hasOwnProperty(propKey) && - typeof propTypes[propKey] === "function" - ) { - let err = propTypes[propKey](props, propKey, name, "prop"); - if (err) console.error(new Error(err.message || err)); - } - } + let props = vnode.props.children + ? vnode.props + : { + ...vnode.props, + ...{ + children: Array.isArray(vnode.children) && + vnode.children.length === 1 + ? vnode.children[0] + : vnode.children + } + }; + + PropTypes.checkPropTypes(propTypes, props, 'prop', name); } } } diff --git a/test/component.js b/test/component.js index b259388..e48af9a 100644 --- a/test/component.js +++ b/test/component.js @@ -244,28 +244,6 @@ describe('components', () => { }; checkPropTypes(Bip, 'Bip'); }); - - it.only('should handle function as children propTypes', () => { - function Foo (props) { return React.Children.only(props.children)(); } - - Foo.propTypes = { - children: React.PropTypes.func.isRequired - }; - - sinon.stub(console, 'error'); - - React.render(, scratch); - expect(console.error).to.have.been.calledWithMatch({ - message: 'Children.only() expects only one child.' - }); - - console.error.reset(); - - React.render({() =>
}, scratch); - expect(console.error).not.to.have.been.called; - - console.error.restore(); - }); }); describe("mixins", () => { From 2ba226929b178eb42c6cfe9df811c73a37729838 Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Fri, 14 Apr 2017 22:06:32 -0600 Subject: [PATCH 7/8] Add test case for prop type checking on cloned elements. #350 --- test/component.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/component.js b/test/component.js index e48af9a..d290d68 100644 --- a/test/component.js +++ b/test/component.js @@ -193,11 +193,11 @@ describe('components', () => { console.error.reset(); - const clone = React.cloneElement( {}} bool="one"/>); + const clone = React.cloneElement(); React.render(clone, scratch); - expect(console.error).to.have.been.calledWithMatch({ - message: 'Invalid prop `bool` of type `string` supplied to `Foo`, expected `boolean`.' - }); + expect(console.error).to.have.been.calledWithMatch( + 'Warning: Failed prop type: Invalid prop `func` of type `string` supplied to `' + name + '`, expected `function`.' + ); console.error.restore(); } From 22f45e450341805e7b0ed23aeb8da2f022fcd50c Mon Sep 17 00:00:00 2001 From: Kye Hohenberger Date: Fri, 14 Apr 2017 22:16:58 -0600 Subject: [PATCH 8/8] Properly format children by running props through propHooks. --- src/index.js | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/index.js b/src/index.js index d93a1ae..ef298f9 100644 --- a/src/index.js +++ b/src/index.js @@ -280,18 +280,15 @@ function validatePropTypes (vnode) { let name = componentClass.displayName || componentClass.name; let propTypes = componentClass.propTypes; if (propTypes) { - let props = vnode.props.children - ? vnode.props - : { - ...vnode.props, - ...{ - children: Array.isArray(vnode.children) && - vnode.children.length === 1 - ? vnode.children[0] - : vnode.children - } - }; - + let props = vnode.props; + if ( + !(vnode.props && vnode.props.children) && + vnode.children && + vnode.children.length + ) { + props.children = vnode.children; + } + propsHook(props); PropTypes.checkPropTypes(propTypes, props, 'prop', name); } }