From 9ed0c4c584771170de585578e4ddefdcb8dc810b Mon Sep 17 00:00:00 2001 From: Tae Kim Date: Wed, 20 Mar 2019 00:11:30 -0700 Subject: [PATCH] Validate extending styles --- src/ThemedStyleSheet.js | 59 +++++++++-- src/withStyles.jsx | 5 +- test/withExtendStyles_test.jsx | 176 +++++++++++++++++++++++++++------ test/withStyles_test.jsx | 99 ++++++++++++++++--- 4 files changed, 283 insertions(+), 56 deletions(-) diff --git a/src/ThemedStyleSheet.js b/src/ThemedStyleSheet.js index 049aa147..5eac231a 100644 --- a/src/ThemedStyleSheet.js +++ b/src/ThemedStyleSheet.js @@ -11,25 +11,68 @@ function registerInterface(interfaceToRegister) { styleInterface = interfaceToRegister; } -function extendStyles(makeFromTheme, extendFromTheme = []) { +function validateStyle(style, extendableStyles, currentPath = '') { + if (style === null || Array.isArray(style) || typeof style !== 'object') { + return; + } + + const styleKeys = Object.keys(style); + if (styleKeys.length) { + styleKeys.forEach((styleKey) => { + const path = `${currentPath}.${styleKey}`; + const isValid = extendableStyles[styleKey]; + if (!isValid) { + throw new Error( + `withStyles() extending style is invalid: ${path}. If this style is expected, add it to` + + 'the component\'s "extendableStyles" option.', + ); + } + validateStyle(style[styleKey], extendableStyles[styleKey], path); + }); + } +} + +function validateAndMergeStyles(makeFromTheme, extendFromTheme = [], extendableStyles) { const baseStyle = makeFromTheme(styleTheme); - const extendedStyles = extendFromTheme.map(extendStyleFn => extendStyleFn(styleTheme)); + const extendedStyles = extendFromTheme.map((extendStyleFn) => { + const style = extendStyleFn(styleTheme); + + if (process.env.NODE_ENV !== 'production') { + validateStyle(style, extendableStyles); + } + + return style; + }); return deepmerge.all([baseStyle, ...extendedStyles]); } -function create(makeFromTheme, createWithDirection, extendFromTheme) { - const styles = createWithDirection(extendStyles(makeFromTheme, extendFromTheme)); +function create(makeFromTheme, createWithDirection, extendFromTheme, extendableStyles) { + const styles = createWithDirection(validateAndMergeStyles( + makeFromTheme, + extendFromTheme, + extendableStyles, + )); return () => styles; } -function createLTR(makeFromTheme, extendFromTheme) { - return create(makeFromTheme, styleInterface.createLTR || styleInterface.create, extendFromTheme); +function createLTR(makeFromTheme, extendFromTheme, extendableStyles) { + return create( + makeFromTheme, + styleInterface.createLTR || styleInterface.create, + extendFromTheme, + extendableStyles, + ); } -function createRTL(makeFromTheme, extendFromTheme) { - return create(makeFromTheme, styleInterface.createRTL || styleInterface.create, extendFromTheme); +function createRTL(makeFromTheme, extendFromTheme, extendableStyles) { + return create( + makeFromTheme, + styleInterface.createRTL || styleInterface.create, + extendFromTheme, + extendableStyles, + ); } function get() { diff --git a/src/withStyles.jsx b/src/withStyles.jsx index e580732c..4b58e479 100644 --- a/src/withStyles.jsx +++ b/src/withStyles.jsx @@ -48,6 +48,7 @@ export function withStyles( themePropName = 'theme', cssPropName = 'css', extendStyleFnPropName = '_extendStyleFn', + extendableStyles = {}, flushBefore = false, pureComponent = false, } = {}, @@ -97,14 +98,14 @@ export function withStyles( if (isRTL) { styleDefRTL = styleFn - ? ThemedStyleSheet.createRTL(styleFn, extendStyleFns) + ? ThemedStyleSheet.createRTL(styleFn, extendStyleFns, extendableStyles) : EMPTY_STYLES_FN; currentThemeRTL = registeredTheme; styleDef = styleDefRTL; } else { styleDefLTR = styleFn - ? ThemedStyleSheet.createLTR(styleFn, extendStyleFns) + ? ThemedStyleSheet.createLTR(styleFn, extendStyleFns, extendableStyles) : EMPTY_STYLES_FN; currentThemeLTR = registeredTheme; diff --git a/test/withExtendStyles_test.jsx b/test/withExtendStyles_test.jsx index 0f35e05d..7d8010b9 100644 --- a/test/withExtendStyles_test.jsx +++ b/test/withExtendStyles_test.jsx @@ -95,12 +95,21 @@ describe('withExtendStyles()', () => { return null; } - const WrappedComponent = withStyles(() => ({ - container: { - background: 'red', - color: 'blue', + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({ container: { background: 'green', @@ -124,19 +133,34 @@ describe('withExtendStyles()', () => { return null; } - const WrappedComponent = withStyles(() => ({ - container: { - background: 'red', - color: 'blue', - }, - innerContainer: { - background: 'white', - border: '1px solid black', - }, - content: { - fontSize: '25px', + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + }, + innerContainer: { + background: 'white', + border: '1px solid black', + }, + content: { + fontSize: '25px', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + innerContainer: { + border: true, + }, + content: { + fontSize: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({ container: { background: 'green', @@ -173,13 +197,23 @@ describe('withExtendStyles()', () => { return null; } - const WrappedComponent = withStyles(() => ({ - container: { - background: 'red', - color: 'blue', - fontSize: '10px', + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + fontSize: '10px', + }, + }), + { + extendableStyles: { + container: { + background: true, + fontSize: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({ container: { @@ -188,14 +222,14 @@ describe('withExtendStyles()', () => { }, }))(WrappedComponent); - const WrappedComponentWithNestedCustomStyles = withExtendStyles(() => ({ + const WrappedComponentWithNestedExtendedStyles = withExtendStyles(() => ({ container: { fontSize: '20px', }, }))(WrappedComponentWithExtendedStyles); render( - , + , ); expect(testInterface.createLTR.callCount).to.equal(1); @@ -208,7 +242,71 @@ describe('withExtendStyles()', () => { }); }); - it('uses original base styles if no extended styles are provided', () => { + it('throws an error if an invalid extending style is provided', () => { + function MyComponent() { + return null; + } + + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + fontSize: '10px', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + }, + }, + )(MyComponent); + + const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({ + container: { + // fontSize is invalid + fontSize: '12px', + }, + }))(WrappedComponent); + + expect(() => render( + , + )).to.throw(); + }); + + it('throws an error if extendableStyles is not defined, and an extending style is provided', () => { + function MyComponent() { + return null; + } + + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + fontSize: '10px', + }, + }), + { + // no extendableStyles + }, + )(MyComponent); + + const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({ + container: { + // fontSize is invalid + fontSize: '12px', + }, + }))(WrappedComponent); + + expect(() => render( + , + )).to.throw(); + }); + + it('uses original base styles if no extending styles are provided', () => { function MyComponent() { return null; } @@ -261,7 +359,14 @@ describe('withExtendStyles()', () => { color: 'blue', }, }), - { extendStyleFnPropName: 'foobar' }, + { + extendStyleFnPropName: 'foobar', + extendableStyles: { + container: { + background: true, + }, + }, + }, )(MyComponent); const WrappedComponentWithExtendedStyles = withExtendStyles( () => ({ @@ -290,11 +395,20 @@ describe('withExtendStyles()', () => { return null; } - const WrappedComponent = withStyles(({ color }) => ({ - foo: { - color: color.red, + const WrappedComponent = withStyles( + ({ color }) => ({ + foo: { + color: color.red, + }, + }), + { + extendableStyles: { + foo: { + color: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); const WrappedComponentWithExtendedStyles = withExtendStyles(({ color }) => ({ foo: { color: color.blue, diff --git a/test/withStyles_test.jsx b/test/withStyles_test.jsx index 52e3c771..e38b7e7e 100644 --- a/test/withStyles_test.jsx +++ b/test/withStyles_test.jsx @@ -206,12 +206,21 @@ describe('withStyles()', () => { return null; } - const WrappedComponent = withStyles(() => ({ - container: { - background: 'red', - color: 'blue', + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); render( { return null; } - const WrappedComponent = withStyles(() => ({ - container: { - background: 'red', - color: 'blue', + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); render( { return null; } - const WrappedComponent = withStyles(() => ({ - container: { - background: 'red', - color: 'blue', + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + }, }, - }))(MyComponent); + )(MyComponent); render( { }); }); + it('throws an error if an invalid style is extending', () => { + function MyComponent() { + return null; + } + + const WrappedComponent = withStyles( + () => ({ + container: { + background: 'red', + color: 'blue', + }, + }), + { + extendableStyles: { + container: { + background: true, + }, + }, + }, + )(MyComponent); + + expect(() => render( + + ({ + container: { + // color is invalid + color: 'green', + }, + }), + ]} + /> + , + )).to.throw(); + }); + it('receives the registered theme in the extend style function', (done) => { function MyComponent() { return null; @@ -329,6 +393,11 @@ describe('withStyles()', () => { () => ({}), { extendStyleFnPropName: 'newExtendStyleFn', + extendableStyles: { + container: { + background: true, + }, + }, }, )(MyComponent); shallow(