Skip to content

Commit a12cd13

Browse files
TaeKimJRTaeKimJR
authored
Merge pull request #1 from TaeKimJR/tkjr-validate-custom-styles
Validate extending styles
2 parents dd30606 + 9ed0c4c commit a12cd13

File tree

4 files changed

+283
-56
lines changed

4 files changed

+283
-56
lines changed

‎src/ThemedStyleSheet.js‎

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,68 @@ function registerInterface(interfaceToRegister) {
1111
styleInterface = interfaceToRegister;
1212
}
1313

14-
function extendStyles(makeFromTheme, extendFromTheme = []) {
14+
function validateStyle(style, extendableStyles, currentPath = '') {
15+
if (style === null || Array.isArray(style) || typeof style !== 'object') {
16+
return;
17+
}
18+
19+
const styleKeys = Object.keys(style);
20+
if (styleKeys.length) {
21+
styleKeys.forEach((styleKey) => {
22+
const path = `${currentPath}.${styleKey}`;
23+
const isValid = extendableStyles[styleKey];
24+
if (!isValid) {
25+
throw new Error(
26+
`withStyles() extending style is invalid: ${path}. If this style is expected, add it to`
27+
+ 'the component\'s "extendableStyles" option.',
28+
);
29+
}
30+
validateStyle(style[styleKey], extendableStyles[styleKey], path);
31+
});
32+
}
33+
}
34+
35+
function validateAndMergeStyles(makeFromTheme, extendFromTheme = [], extendableStyles) {
1536
const baseStyle = makeFromTheme(styleTheme);
16-
const extendedStyles = extendFromTheme.map(extendStyleFn => extendStyleFn(styleTheme));
37+
const extendedStyles = extendFromTheme.map((extendStyleFn) => {
38+
const style = extendStyleFn(styleTheme);
39+
40+
if (process.env.NODE_ENV !== 'production') {
41+
validateStyle(style, extendableStyles);
42+
}
43+
44+
return style;
45+
});
1746

1847
return deepmerge.all([baseStyle, ...extendedStyles]);
1948
}
2049

21-
function create(makeFromTheme, createWithDirection, extendFromTheme) {
22-
const styles = createWithDirection(extendStyles(makeFromTheme, extendFromTheme));
50+
function create(makeFromTheme, createWithDirection, extendFromTheme, extendableStyles) {
51+
const styles = createWithDirection(validateAndMergeStyles(
52+
makeFromTheme,
53+
extendFromTheme,
54+
extendableStyles,
55+
));
2356

2457
return () => styles;
2558
}
2659

27-
function createLTR(makeFromTheme, extendFromTheme) {
28-
return create(makeFromTheme, styleInterface.createLTR || styleInterface.create, extendFromTheme);
60+
function createLTR(makeFromTheme, extendFromTheme, extendableStyles) {
61+
return create(
62+
makeFromTheme,
63+
styleInterface.createLTR || styleInterface.create,
64+
extendFromTheme,
65+
extendableStyles,
66+
);
2967
}
3068

31-
function createRTL(makeFromTheme, extendFromTheme) {
32-
return create(makeFromTheme, styleInterface.createRTL || styleInterface.create, extendFromTheme);
69+
function createRTL(makeFromTheme, extendFromTheme, extendableStyles) {
70+
return create(
71+
makeFromTheme,
72+
styleInterface.createRTL || styleInterface.create,
73+
extendFromTheme,
74+
extendableStyles,
75+
);
3376
}
3477

3578
function get() {

‎src/withStyles.jsx‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export function withStyles(
4848
themePropName = 'theme',
4949
cssPropName = 'css',
5050
extendStyleFnPropName = '_extendStyleFn',
51+
extendableStyles = {},
5152
flushBefore = false,
5253
pureComponent = false,
5354
} = {},
@@ -97,14 +98,14 @@ export function withStyles(
9798

9899
if (isRTL) {
99100
styleDefRTL = styleFn
100-
? ThemedStyleSheet.createRTL(styleFn, extendStyleFns)
101+
? ThemedStyleSheet.createRTL(styleFn, extendStyleFns, extendableStyles)
101102
: EMPTY_STYLES_FN;
102103

103104
currentThemeRTL = registeredTheme;
104105
styleDef = styleDefRTL;
105106
} else {
106107
styleDefLTR = styleFn
107-
? ThemedStyleSheet.createLTR(styleFn, extendStyleFns)
108+
? ThemedStyleSheet.createLTR(styleFn, extendStyleFns, extendableStyles)
108109
: EMPTY_STYLES_FN;
109110

110111
currentThemeLTR = registeredTheme;

‎test/withExtendStyles_test.jsx‎

Lines changed: 145 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -95,12 +95,21 @@ describe('withExtendStyles()', () => {
9595
return null;
9696
}
9797

98-
const WrappedComponent = withStyles(() => ({
99-
container: {
100-
background: 'red',
101-
color: 'blue',
98+
const WrappedComponent = withStyles(
99+
() => ({
100+
container: {
101+
background: 'red',
102+
color: 'blue',
103+
},
104+
}),
105+
{
106+
extendableStyles: {
107+
container: {
108+
background: true,
109+
},
110+
},
102111
},
103-
}))(MyComponent);
112+
)(MyComponent);
104113
const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({
105114
container: {
106115
background: 'green',
@@ -124,19 +133,34 @@ describe('withExtendStyles()', () => {
124133
return null;
125134
}
126135

127-
const WrappedComponent = withStyles(() => ({
128-
container: {
129-
background: 'red',
130-
color: 'blue',
131-
},
132-
innerContainer: {
133-
background: 'white',
134-
border: '1px solid black',
135-
},
136-
content: {
137-
fontSize: '25px',
136+
const WrappedComponent = withStyles(
137+
() => ({
138+
container: {
139+
background: 'red',
140+
color: 'blue',
141+
},
142+
innerContainer: {
143+
background: 'white',
144+
border: '1px solid black',
145+
},
146+
content: {
147+
fontSize: '25px',
148+
},
149+
}),
150+
{
151+
extendableStyles: {
152+
container: {
153+
background: true,
154+
},
155+
innerContainer: {
156+
border: true,
157+
},
158+
content: {
159+
fontSize: true,
160+
},
161+
},
138162
},
139-
}))(MyComponent);
163+
)(MyComponent);
140164
const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({
141165
container: {
142166
background: 'green',
@@ -173,13 +197,23 @@ describe('withExtendStyles()', () => {
173197
return null;
174198
}
175199

176-
const WrappedComponent = withStyles(() => ({
177-
container: {
178-
background: 'red',
179-
color: 'blue',
180-
fontSize: '10px',
200+
const WrappedComponent = withStyles(
201+
() => ({
202+
container: {
203+
background: 'red',
204+
color: 'blue',
205+
fontSize: '10px',
206+
},
207+
}),
208+
{
209+
extendableStyles: {
210+
container: {
211+
background: true,
212+
fontSize: true,
213+
},
214+
},
181215
},
182-
}))(MyComponent);
216+
)(MyComponent);
183217

184218
const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({
185219
container: {
@@ -188,14 +222,14 @@ describe('withExtendStyles()', () => {
188222
},
189223
}))(WrappedComponent);
190224

191-
const WrappedComponentWithNestedCustomStyles = withExtendStyles(() => ({
225+
const WrappedComponentWithNestedExtendedStyles = withExtendStyles(() => ({
192226
container: {
193227
fontSize: '20px',
194228
},
195229
}))(WrappedComponentWithExtendedStyles);
196230

197231
render(
198-
<WrappedComponentWithNestedCustomStyles />,
232+
<WrappedComponentWithNestedExtendedStyles />,
199233
);
200234

201235
expect(testInterface.createLTR.callCount).to.equal(1);
@@ -208,7 +242,71 @@ describe('withExtendStyles()', () => {
208242
});
209243
});
210244

211-
it('uses original base styles if no extended styles are provided', () => {
245+
it('throws an error if an invalid extending style is provided', () => {
246+
function MyComponent() {
247+
return null;
248+
}
249+
250+
const WrappedComponent = withStyles(
251+
() => ({
252+
container: {
253+
background: 'red',
254+
color: 'blue',
255+
fontSize: '10px',
256+
},
257+
}),
258+
{
259+
extendableStyles: {
260+
container: {
261+
background: true,
262+
},
263+
},
264+
},
265+
)(MyComponent);
266+
267+
const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({
268+
container: {
269+
// fontSize is invalid
270+
fontSize: '12px',
271+
},
272+
}))(WrappedComponent);
273+
274+
expect(() => render(
275+
<WrappedComponentWithExtendedStyles />,
276+
)).to.throw();
277+
});
278+
279+
it('throws an error if extendableStyles is not defined, and an extending style is provided', () => {
280+
function MyComponent() {
281+
return null;
282+
}
283+
284+
const WrappedComponent = withStyles(
285+
() => ({
286+
container: {
287+
background: 'red',
288+
color: 'blue',
289+
fontSize: '10px',
290+
},
291+
}),
292+
{
293+
// no extendableStyles
294+
},
295+
)(MyComponent);
296+
297+
const WrappedComponentWithExtendedStyles = withExtendStyles(() => ({
298+
container: {
299+
// fontSize is invalid
300+
fontSize: '12px',
301+
},
302+
}))(WrappedComponent);
303+
304+
expect(() => render(
305+
<WrappedComponentWithExtendedStyles />,
306+
)).to.throw();
307+
});
308+
309+
it('uses original base styles if no extending styles are provided', () => {
212310
function MyComponent() {
213311
return null;
214312
}
@@ -261,7 +359,14 @@ describe('withExtendStyles()', () => {
261359
color: 'blue',
262360
},
263361
}),
264-
{ extendStyleFnPropName: 'foobar' },
362+
{
363+
extendStyleFnPropName: 'foobar',
364+
extendableStyles: {
365+
container: {
366+
background: true,
367+
},
368+
},
369+
},
265370
)(MyComponent);
266371
const WrappedComponentWithExtendedStyles = withExtendStyles(
267372
() => ({
@@ -290,11 +395,20 @@ describe('withExtendStyles()', () => {
290395
return null;
291396
}
292397

293-
const WrappedComponent = withStyles(({ color }) => ({
294-
foo: {
295-
color: color.red,
398+
const WrappedComponent = withStyles(
399+
({ color }) => ({
400+
foo: {
401+
color: color.red,
402+
},
403+
}),
404+
{
405+
extendableStyles: {
406+
foo: {
407+
color: true,
408+
},
409+
},
296410
},
297-
}))(MyComponent);
411+
)(MyComponent);
298412
const WrappedComponentWithExtendedStyles = withExtendStyles(({ color }) => ({
299413
foo: {
300414
color: color.blue,

0 commit comments

Comments
 (0)