From 12f5d969d557a952bd047b4125990af006bc7de5 Mon Sep 17 00:00:00 2001 From: D V Date: Wed, 20 Sep 2023 14:53:22 +0530 Subject: [PATCH 1/5] update Typography color prop script and test --- packages/mui-material/src/Typography/Typography.test.js | 9 +++++++++ packages/mui-system/src/style.js | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/packages/mui-material/src/Typography/Typography.test.js b/packages/mui-material/src/Typography/Typography.test.js index e6877e3339bb57..0144dbd0f2b9f4 100644 --- a/packages/mui-material/src/Typography/Typography.test.js +++ b/packages/mui-material/src/Typography/Typography.test.js @@ -122,4 +122,13 @@ describe('', () => { marginBottom: '16px', }); }); + + it('should check for invalid color value', () => { + const { container, debug } = render( + + Hello + , + ); + debug(container); + }); }); diff --git a/packages/mui-system/src/style.js b/packages/mui-system/src/style.js index 2767e7e306623a..144b59ae4dfe1d 100644 --- a/packages/mui-system/src/style.js +++ b/packages/mui-system/src/style.js @@ -39,6 +39,14 @@ export function getStyleValue(themeMapping, transform, propValueFinal, userValue value = transform(value, userValue, themeMapping); } + if (typeof value !== 'string' && typeof value !== 'number' && typeof value !== 'boolean') { + // Log an error in development mode + if (process.env.NODE_ENV !== 'production') { + console.error(`Invalid value "${value}" for property "${userValue}"`); + } + value = userValue; + } + return value; } From 0e1ea8e0c2d0a1e07371e3d09479e168237e9d13 Mon Sep 17 00:00:00 2001 From: D V Date: Wed, 20 Sep 2023 21:36:50 +0530 Subject: [PATCH 2/5] add Typography prop color invalid value test --- .../src/Typography/Typography.test.js | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/packages/mui-material/src/Typography/Typography.test.js b/packages/mui-material/src/Typography/Typography.test.js index 0144dbd0f2b9f4..672a0322a2f1f3 100644 --- a/packages/mui-material/src/Typography/Typography.test.js +++ b/packages/mui-material/src/Typography/Typography.test.js @@ -3,6 +3,7 @@ import * as React from 'react'; import { expect } from 'chai'; import { createRenderer, describeConformance } from 'test/utils'; import Typography, { typographyClasses as classes } from '@mui/material/Typography'; +import sinon from 'sinon'; describe('', () => { const { render } = createRenderer(); @@ -112,6 +113,21 @@ describe('', () => { }); }); + describe('prop: color', () => { + it('should check for invalid color value', () => { + const consoleErrorStub = sinon.stub(console, 'error'); + + render( + + Hello + , + ); + + sinon.assert.called(consoleErrorStub); + consoleErrorStub.restore(); + }); + }); + it('combines system properties with the sx prop', () => { const { container } = render(); @@ -122,13 +138,4 @@ describe('', () => { marginBottom: '16px', }); }); - - it('should check for invalid color value', () => { - const { container, debug } = render( - - Hello - , - ); - debug(container); - }); }); From 04e6b616bb0e2abd5fd2225bc9651f719aba06f1 Mon Sep 17 00:00:00 2001 From: D V Date: Mon, 25 Sep 2023 23:38:20 +0530 Subject: [PATCH 3/5] update getStyleValue and add tests --- .../src/Typography/Typography.test.js | 10 +++++-- packages/mui-system/src/style.js | 15 +++++----- packages/mui-system/src/style.test.js | 29 ++++++++++++++++++- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/packages/mui-material/src/Typography/Typography.test.js b/packages/mui-material/src/Typography/Typography.test.js index 672a0322a2f1f3..138a7ce5754949 100644 --- a/packages/mui-material/src/Typography/Typography.test.js +++ b/packages/mui-material/src/Typography/Typography.test.js @@ -115,7 +115,7 @@ describe('', () => { describe('prop: color', () => { it('should check for invalid color value', () => { - const consoleErrorStub = sinon.stub(console, 'error'); + const consoleWarnStub = sinon.stub(console, 'warn'); render( @@ -123,8 +123,12 @@ describe('', () => { , ); - sinon.assert.called(consoleErrorStub); - consoleErrorStub.restore(); + sinon.assert.calledWith( + consoleWarnStub, + 'MUI: Invalid value found in theme for prop: "background". It should be a string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + ); + + consoleWarnStub.restore(); }); }); diff --git a/packages/mui-system/src/style.js b/packages/mui-system/src/style.js index 144b59ae4dfe1d..a51bcb39b1a6aa 100644 --- a/packages/mui-system/src/style.js +++ b/packages/mui-system/src/style.js @@ -35,18 +35,19 @@ export function getStyleValue(themeMapping, transform, propValueFinal, userValue value = getPath(themeMapping, propValueFinal) || userValue; } - if (transform) { - value = transform(value, userValue, themeMapping); - } - - if (typeof value !== 'string' && typeof value !== 'number' && typeof value !== 'boolean') { - // Log an error in development mode + if (typeof value === 'object') { if (process.env.NODE_ENV !== 'production') { - console.error(`Invalid value "${value}" for property "${userValue}"`); + console.warn( + `MUI: Invalid value found in theme for prop: "${propValueFinal}". It should be a string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".`, + ); } value = userValue; } + if (transform) { + value = transform(value, userValue, themeMapping); + } + return value; } diff --git a/packages/mui-system/src/style.test.js b/packages/mui-system/src/style.test.js index 618017cbd214ff..4ca041c6d0296d 100644 --- a/packages/mui-system/src/style.test.js +++ b/packages/mui-system/src/style.test.js @@ -1,5 +1,6 @@ import { expect } from 'chai'; -import style from './style'; +import sinon from 'sinon'; +import style, { getStyleValue } from './style'; describe('style', () => { const bgcolor = style({ @@ -257,5 +258,31 @@ describe('style', () => { opacity: 0.5, }); }); + + it('should warn', () => { + const round = (value) => Math.round(value * 1e5) / 1e5; + const consoleWarnStub = sinon.stub(console, 'warn'); + + getStyleValue( + { + body1: { + fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', + fontSize: '1rem', + letterSpacing: `${round(0.15 / 16)}em`, + fontWeight: 400, + lineHeight: 1.5, + }, + }, + null, + 'body1', + ); + + sinon.assert.calledWith( + consoleWarnStub, + 'MUI: Invalid value found in theme for prop: "body1". It should be a string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + ); + + consoleWarnStub.restore(); + }); }); }); From 71a5746578d987846e5a93e7d11ad01fe9669e90 Mon Sep 17 00:00:00 2001 From: D V Date: Thu, 28 Sep 2023 02:18:19 +0530 Subject: [PATCH 4/5] fix code and add more tests --- .../src/Typography/Typography.test.js | 2 +- packages/mui-system/src/palette.test.js | 11 +++++ packages/mui-system/src/style.js | 3 +- packages/mui-system/src/style.test.js | 44 +++++++++++++++++-- .../styleFunctionSx/styleFunctionSx.test.js | 6 +++ 5 files changed, 59 insertions(+), 7 deletions(-) diff --git a/packages/mui-material/src/Typography/Typography.test.js b/packages/mui-material/src/Typography/Typography.test.js index 138a7ce5754949..214cf1668a677d 100644 --- a/packages/mui-material/src/Typography/Typography.test.js +++ b/packages/mui-material/src/Typography/Typography.test.js @@ -125,7 +125,7 @@ describe('', () => { sinon.assert.calledWith( consoleWarnStub, - 'MUI: Invalid value found in theme for prop: "background". It should be a string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + 'MUI: The value found in theme for prop: "background" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', ); consoleWarnStub.restore(); diff --git a/packages/mui-system/src/palette.test.js b/packages/mui-system/src/palette.test.js index 3c12032546dba8..4456da704e5714 100644 --- a/packages/mui-system/src/palette.test.js +++ b/packages/mui-system/src/palette.test.js @@ -1,4 +1,5 @@ import { expect } from 'chai'; +import sinon from 'sinon'; import palette from './palette'; const theme = { @@ -9,10 +10,20 @@ const theme = { describe('palette', () => { it('should treat grey as CSS color', () => { + const consoleWarnStub = sinon.stub(console, 'warn'); + const output = palette({ theme, backgroundColor: 'grey', }); + + sinon.assert.calledWith( + consoleWarnStub, + 'MUI: The value found in theme for prop: "grey" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + ); + + consoleWarnStub.restore(); + expect(output).to.deep.equal({ backgroundColor: 'grey', }); diff --git a/packages/mui-system/src/style.js b/packages/mui-system/src/style.js index a51bcb39b1a6aa..9ee5757ed90caa 100644 --- a/packages/mui-system/src/style.js +++ b/packages/mui-system/src/style.js @@ -38,10 +38,9 @@ export function getStyleValue(themeMapping, transform, propValueFinal, userValue if (typeof value === 'object') { if (process.env.NODE_ENV !== 'production') { console.warn( - `MUI: Invalid value found in theme for prop: "${propValueFinal}". It should be a string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".`, + `MUI: The value found in theme for prop: "${propValueFinal}" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".`, ); } - value = userValue; } if (transform) { diff --git a/packages/mui-system/src/style.test.js b/packages/mui-system/src/style.test.js index 4ca041c6d0296d..6a72aec8892612 100644 --- a/packages/mui-system/src/style.test.js +++ b/packages/mui-system/src/style.test.js @@ -258,12 +258,13 @@ describe('style', () => { opacity: 0.5, }); }); - - it('should warn', () => { + }); + describe('getStyleValue', () => { + it('should warn on acceptable object', () => { const round = (value) => Math.round(value * 1e5) / 1e5; const consoleWarnStub = sinon.stub(console, 'warn'); - getStyleValue( + const output = getStyleValue( { body1: { fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', @@ -279,10 +280,45 @@ describe('style', () => { sinon.assert.calledWith( consoleWarnStub, - 'MUI: Invalid value found in theme for prop: "body1". It should be a string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + 'MUI: The value found in theme for prop: "body1" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + ); + + consoleWarnStub.restore(); + + expect(output).to.deep.equal({ + fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', + fontSize: '1rem', + letterSpacing: `${round(0.15 / 16)}em`, + fontWeight: 400, + lineHeight: 1.5, + }); + }); + + it('should warn on unacceptable object', () => { + const theme = { + palette: { + grey: { 100: '#f5f5f5' }, + }, + }; + + const paletteTransform = (value, userValue) => { + if (userValue === 'grey') { + return userValue; + } + return value; + }; + const consoleWarnStub = sinon.stub(console, 'warn'); + + const output = getStyleValue(theme.palette, paletteTransform, 'grey'); + + sinon.assert.calledWith( + consoleWarnStub, + 'MUI: The value found in theme for prop: "grey" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', ); consoleWarnStub.restore(); + + expect(output).to.be.equal('grey'); }); }); }); diff --git a/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js b/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js index ba0d89c8fdb8f6..bc90ac06bf35ba 100644 --- a/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js +++ b/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js @@ -1,6 +1,7 @@ import { expect } from 'chai'; import createMixins from '@mui/material/styles/createMixins'; import createTypography from '@mui/material/styles/createTypography'; +import sinon from 'sinon'; import createBreakpoints from '../createTheme/createBreakpoints'; import styleFunctionSx from './styleFunctionSx'; @@ -93,11 +94,16 @@ describe('styleFunctionSx', () => { }); it('resolves system typography', () => { + const consoleWarnStub = sinon.stub(console, 'warn'); + const result = styleFunctionSx({ theme, sx: { typography: ['body2', 'body1'] }, }); + sinon.assert.calledTwice(consoleWarnStub); + consoleWarnStub.restore(); + expect(result).to.deep.equal({ '@media (min-width:0px)': { fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', From 3fb84bb4e8714340bd71476e75be0b92040fa693 Mon Sep 17 00:00:00 2001 From: D V Date: Fri, 29 Sep 2023 13:05:18 +0530 Subject: [PATCH 5/5] use toWarnDev instead of sinon stubs --- .../src/Typography/Typography.test.js | 24 ++++------ packages/mui-system/src/palette.test.js | 19 +++----- packages/mui-system/src/style.test.js | 47 ++++++++----------- .../styleFunctionSx/styleFunctionSx.test.js | 19 ++++---- 4 files changed, 46 insertions(+), 63 deletions(-) diff --git a/packages/mui-material/src/Typography/Typography.test.js b/packages/mui-material/src/Typography/Typography.test.js index 4b4aa6be6f87c8..0012276b60ac6f 100644 --- a/packages/mui-material/src/Typography/Typography.test.js +++ b/packages/mui-material/src/Typography/Typography.test.js @@ -3,7 +3,6 @@ import * as React from 'react'; import { expect } from 'chai'; import { createRenderer, describeConformance } from '@mui-internal/test-utils'; import Typography, { typographyClasses as classes } from '@mui/material/Typography'; -import sinon from 'sinon'; describe('', () => { const { render } = createRenderer(); @@ -115,20 +114,15 @@ describe('', () => { describe('prop: color', () => { it('should check for invalid color value', () => { - const consoleWarnStub = sinon.stub(console, 'warn'); - - render( - - Hello - , - ); - - sinon.assert.calledWith( - consoleWarnStub, - 'MUI: The value found in theme for prop: "background" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', - ); - - consoleWarnStub.restore(); + const msg = + 'MUI: The value found in theme for prop: "background" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".'; + expect(() => { + render( + + Hello + , + ); + }).toWarnDev([msg, msg]); }); }); diff --git a/packages/mui-system/src/palette.test.js b/packages/mui-system/src/palette.test.js index 4456da704e5714..bb2fc9ecb56f73 100644 --- a/packages/mui-system/src/palette.test.js +++ b/packages/mui-system/src/palette.test.js @@ -1,5 +1,4 @@ import { expect } from 'chai'; -import sinon from 'sinon'; import palette from './palette'; const theme = { @@ -10,20 +9,16 @@ const theme = { describe('palette', () => { it('should treat grey as CSS color', () => { - const consoleWarnStub = sinon.stub(console, 'warn'); - - const output = palette({ - theme, - backgroundColor: 'grey', - }); - - sinon.assert.calledWith( - consoleWarnStub, + let output; + expect(() => { + output = palette({ + theme, + backgroundColor: 'grey', + }); + }).toWarnDev( 'MUI: The value found in theme for prop: "grey" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', ); - consoleWarnStub.restore(); - expect(output).to.deep.equal({ backgroundColor: 'grey', }); diff --git a/packages/mui-system/src/style.test.js b/packages/mui-system/src/style.test.js index 6a72aec8892612..c5635bfc6789c3 100644 --- a/packages/mui-system/src/style.test.js +++ b/packages/mui-system/src/style.test.js @@ -1,5 +1,4 @@ import { expect } from 'chai'; -import sinon from 'sinon'; import style, { getStyleValue } from './style'; describe('style', () => { @@ -262,29 +261,26 @@ describe('style', () => { describe('getStyleValue', () => { it('should warn on acceptable object', () => { const round = (value) => Math.round(value * 1e5) / 1e5; - const consoleWarnStub = sinon.stub(console, 'warn'); - - const output = getStyleValue( - { - body1: { - fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', - fontSize: '1rem', - letterSpacing: `${round(0.15 / 16)}em`, - fontWeight: 400, - lineHeight: 1.5, + let output; + + expect(() => { + output = getStyleValue( + { + body1: { + fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', + fontSize: '1rem', + letterSpacing: `${round(0.15 / 16)}em`, + fontWeight: 400, + lineHeight: 1.5, + }, }, - }, - null, - 'body1', - ); - - sinon.assert.calledWith( - consoleWarnStub, + null, + 'body1', + ); + }).toWarnDev( 'MUI: The value found in theme for prop: "body1" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', ); - consoleWarnStub.restore(); - expect(output).to.deep.equal({ fontFamily: '"Roboto", "Helvetica", "Arial", sans-serif', fontSize: '1rem', @@ -307,17 +303,14 @@ describe('style', () => { } return value; }; - const consoleWarnStub = sinon.stub(console, 'warn'); + let output; - const output = getStyleValue(theme.palette, paletteTransform, 'grey'); - - sinon.assert.calledWith( - consoleWarnStub, + expect(() => { + output = getStyleValue(theme.palette, paletteTransform, 'grey'); + }).toWarnDev( 'MUI: The value found in theme for prop: "grey" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', ); - consoleWarnStub.restore(); - expect(output).to.be.equal('grey'); }); }); diff --git a/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js b/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js index bc90ac06bf35ba..d19b7a24840f5c 100644 --- a/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js +++ b/packages/mui-system/src/styleFunctionSx/styleFunctionSx.test.js @@ -1,7 +1,6 @@ import { expect } from 'chai'; import createMixins from '@mui/material/styles/createMixins'; import createTypography from '@mui/material/styles/createTypography'; -import sinon from 'sinon'; import createBreakpoints from '../createTheme/createBreakpoints'; import styleFunctionSx from './styleFunctionSx'; @@ -94,15 +93,17 @@ describe('styleFunctionSx', () => { }); it('resolves system typography', () => { - const consoleWarnStub = sinon.stub(console, 'warn'); + let result; - const result = styleFunctionSx({ - theme, - sx: { typography: ['body2', 'body1'] }, - }); - - sinon.assert.calledTwice(consoleWarnStub); - consoleWarnStub.restore(); + expect(() => { + result = styleFunctionSx({ + theme, + sx: { typography: ['body2', 'body1'] }, + }); + }).toWarnDev([ + 'MUI: The value found in theme for prop: "body2" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + 'MUI: The value found in theme for prop: "body1" is an [Object] instead of string or number. Check if you forgot to add the correct dotted notation, eg, "background.paper" instead of "background".', + ]); expect(result).to.deep.equal({ '@media (min-width:0px)': {