From 747801feb14c2c4489df89770142e414f1f987fa Mon Sep 17 00:00:00 2001 From: Marija Najdova Date: Thu, 14 Jan 2021 11:04:38 +0100 Subject: [PATCH 1/2] implemented proposal --- .../src/requirePropFactory.d.ts | 4 +++- .../src/requirePropFactory.js | 12 ++++++++-- .../src/requirePropFactory.test.js | 23 +++++++++++++++++++ packages/material-ui/src/Grid/Grid.js | 2 +- 4 files changed, 37 insertions(+), 4 deletions(-) diff --git a/packages/material-ui-utils/src/requirePropFactory.d.ts b/packages/material-ui-utils/src/requirePropFactory.d.ts index 63ae30db9f2831..9aca039e7be7a8 100644 --- a/packages/material-ui-utils/src/requirePropFactory.d.ts +++ b/packages/material-ui-utils/src/requirePropFactory.d.ts @@ -1 +1,3 @@ -export default function requirePropFactory(componentNameInError: string): any; +type Component = (arg: any) => any & { propTypes?: object }; + +export default function requirePropFactory(componentNameInError: string, Component?: Component): any; diff --git a/packages/material-ui-utils/src/requirePropFactory.js b/packages/material-ui-utils/src/requirePropFactory.js index e8e8b24a2a7259..15262e1ab9b941 100644 --- a/packages/material-ui-utils/src/requirePropFactory.js +++ b/packages/material-ui-utils/src/requirePropFactory.js @@ -1,4 +1,4 @@ -export default function requirePropFactory(componentNameInError) { +export default function requirePropFactory(componentNameInError, Component) { if (process.env.NODE_ENV === 'production') { return () => null; } @@ -12,6 +12,14 @@ export default function requirePropFactory(componentNameInError) { ) => { const propFullNameSafe = propFullName || propName; + const defaultPropType = Component?.propTypes?.[propFullNameSafe]; + let defaultPropTypeVal = null; + + if(defaultPropType) { + console.log("Default prop type existed"); + defaultPropTypeVal = defaultPropType(props, propName, componentName, location, propFullName) + } + if (typeof props[propName] !== 'undefined' && !props[requiredProp]) { return new Error( `The prop \`${propFullNameSafe}\` of ` + @@ -19,7 +27,7 @@ export default function requirePropFactory(componentNameInError) { ); } - return null; + return defaultPropTypeVal; }; return requireProp; } diff --git a/packages/material-ui-utils/src/requirePropFactory.test.js b/packages/material-ui-utils/src/requirePropFactory.test.js index f527d088a4e19d..bb5ce3d79dd845 100644 --- a/packages/material-ui-utils/src/requirePropFactory.test.js +++ b/packages/material-ui-utils/src/requirePropFactory.test.js @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import { spy } from 'sinon'; import requirePropFactory from './requirePropFactory'; describe('requirePropFactory', () => { @@ -83,6 +84,28 @@ describe('requirePropFactory', () => { }); }); }); + + it('should chain the proptypes with the default prop types coming from the component', () => { + const Test = () => null; + const mock = spy(); + Test.propTypes = { + test: mock, + }; + + const props = {}; + const propName = 'test'; + props[propName] = true; + + const requireProp = requirePropFactory('Test', Test); + + const result = requireProp('otherProp'); + result(props, propName, undefined, undefined, undefined); + + expect(mock.callCount).to.equal(1); + expect(mock.args[0]).to.deep.equal( + [props, propName, undefined, undefined, undefined], + ); + }); }); }); }); diff --git a/packages/material-ui/src/Grid/Grid.js b/packages/material-ui/src/Grid/Grid.js index ce5c22aa23a4b5..c23b2b0bbbc1f4 100644 --- a/packages/material-ui/src/Grid/Grid.js +++ b/packages/material-ui/src/Grid/Grid.js @@ -424,7 +424,7 @@ Grid.propTypes = { }; if (process.env.NODE_ENV !== 'production') { - const requireProp = requirePropFactory('Grid'); + const requireProp = requirePropFactory('Grid', Grid); // eslint-disable-next-line no-useless-concat Grid['propTypes' + ''] = { // eslint-disable-next-line react/forbid-foreign-prop-types From 6e25d374ba811ee3b833db46835060e56d427b31 Mon Sep 17 00:00:00 2001 From: Marija Najdova Date: Thu, 14 Jan 2021 11:36:29 +0100 Subject: [PATCH 2/2] Sebastian's review --- .../material-ui-utils/src/requirePropFactory.d.ts | 4 ++-- packages/material-ui-utils/src/requirePropFactory.js | 11 +++++------ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/material-ui-utils/src/requirePropFactory.d.ts b/packages/material-ui-utils/src/requirePropFactory.d.ts index 9aca039e7be7a8..b4759fe6393271 100644 --- a/packages/material-ui-utils/src/requirePropFactory.d.ts +++ b/packages/material-ui-utils/src/requirePropFactory.d.ts @@ -1,3 +1,3 @@ -type Component = (arg: any) => any & { propTypes?: object }; +import * as React from 'react'; -export default function requirePropFactory(componentNameInError: string, Component?: Component): any; +export default function requirePropFactory(componentNameInError: string, Component?: React.ComponentType): any; diff --git a/packages/material-ui-utils/src/requirePropFactory.js b/packages/material-ui-utils/src/requirePropFactory.js index 15262e1ab9b941..dd91a063463f8d 100644 --- a/packages/material-ui-utils/src/requirePropFactory.js +++ b/packages/material-ui-utils/src/requirePropFactory.js @@ -12,12 +12,11 @@ export default function requirePropFactory(componentNameInError, Component) { ) => { const propFullNameSafe = propFullName || propName; - const defaultPropType = Component?.propTypes?.[propFullNameSafe]; - let defaultPropTypeVal = null; + const defaultTypeChecker = Component?.propTypes?.[propFullNameSafe]; + let defaultTypeCheckerResult = null; - if(defaultPropType) { - console.log("Default prop type existed"); - defaultPropTypeVal = defaultPropType(props, propName, componentName, location, propFullName) + if(defaultTypeChecker) { + defaultTypeCheckerResult = defaultTypeChecker(props, propName, componentName, location, propFullName) } if (typeof props[propName] !== 'undefined' && !props[requiredProp]) { @@ -27,7 +26,7 @@ export default function requirePropFactory(componentNameInError, Component) { ); } - return defaultPropTypeVal; + return defaultTypeCheckerResult; }; return requireProp; }