Skip to content

Commit

Permalink
[system] Fix all use of styled(Box) (#40449)
Browse files Browse the repository at this point in the history
Signed-off-by: Olivier Tassinari <[email protected]>
  • Loading branch information
oliviertassinari authored Jan 9, 2024
1 parent 48a2922 commit ea16771
Show file tree
Hide file tree
Showing 16 changed files with 155 additions and 29 deletions.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ module.exports = {
'material-ui/docgen-ignore-before-comment': 'error',
'material-ui/rules-of-use-theme-variants': 'error',
'material-ui/no-empty-box': 'error',
'material-ui/no-styled-box': 'error',
'material-ui/straight-quotes': 'error',

'react-hooks/exhaustive-deps': ['error', { additionalHooks: 'useEnhancedEffect' }],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Box from '@mui/joy/Box';
import Button from '@mui/joy/Button';
import Typography from '@mui/joy/Typography';

const StyledBox = styled(Box)(
const StyledBox = styled('div')(
({ theme }) => ({
padding: 32,
display: 'grid',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import Box from '@mui/joy/Box';
import Button from '@mui/joy/Button';
import Typography from '@mui/joy/Typography';

const StyledBox = styled(Box)(
const StyledBox = styled('div')(
({ theme }) => ({
padding: 32,
display: 'grid',
Expand Down
4 changes: 2 additions & 2 deletions docs/data/material/components/drawers/SwipeableEdgeDrawer.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ const Root = styled('div')(({ theme }) => ({
theme.palette.mode === 'light' ? grey[100] : theme.palette.background.default,
}));

const StyledBox = styled(Box)(({ theme }) => ({
const StyledBox = styled('div')(({ theme }) => ({
backgroundColor: theme.palette.mode === 'light' ? '#fff' : grey[800],
}));

const Puller = styled(Box)(({ theme }) => ({
const Puller = styled('div')(({ theme }) => ({
width: 30,
height: 6,
backgroundColor: theme.palette.mode === 'light' ? grey[300] : grey[900],
Expand Down
4 changes: 2 additions & 2 deletions docs/data/material/components/drawers/SwipeableEdgeDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ const Root = styled('div')(({ theme }) => ({
theme.palette.mode === 'light' ? grey[100] : theme.palette.background.default,
}));

const StyledBox = styled(Box)(({ theme }) => ({
const StyledBox = styled('div')(({ theme }) => ({
backgroundColor: theme.palette.mode === 'light' ? '#fff' : grey[800],
}));

const Puller = styled(Box)(({ theme }) => ({
const Puller = styled('div')(({ theme }) => ({
width: 30,
height: 6,
backgroundColor: theme.palette.mode === 'light' ? grey[300] : grey[900],
Expand Down
5 changes: 2 additions & 3 deletions docs/data/material/components/material-icons/SearchIcons.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as React from 'react';
import { styled } from '@mui/material/styles';
import MuiPaper from '@mui/material/Paper';
import Box from '@mui/material/Box';
import copy from 'clipboard-copy';
import InputBase from '@mui/material/InputBase';
import Typography from '@mui/material/Typography';
Expand Down Expand Up @@ -209,7 +208,7 @@ const Title = styled(Typography)(({ theme }) => ({
},
}));

const CanvasComponent = styled(Box)(({ theme }) => ({
const CanvasComponent = styled('div')(({ theme }) => ({
fontSize: 210,
marginTop: theme.spacing(2),
color: theme.palette.text.primary,
Expand All @@ -226,7 +225,7 @@ const FontSizeComponent = styled('span')(({ theme }) => ({
margin: theme.spacing(2),
}));

const ContextComponent = styled(Box, {
const ContextComponent = styled('div', {
shouldForwardProp: (prop) => prop !== 'contextColor',
})(({ theme, contextColor }) => ({
margin: theme.spacing(0.5),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import { experimentalStyled as styled } from '@mui/material/styles';
import Box from '@mui/material/Box';

import Typography from '../components/Typography';

const BoxStyled = styled(Box, {
const Root = styled('div', {
shouldForwardProp: (prop) => prop !== 'error' && prop !== 'success',
})(({ theme, error, success }) => ({
padding: theme.spacing(2),
Expand All @@ -22,9 +22,9 @@ function FormFeedback(props) {
const { className, children, error, success, ...others } = props;

return (
<BoxStyled error={error} success={success} className={className} {...others}>
<Root error={error} success={success} className={className} {...others}>
<Typography color="inherit">{children}</Typography>
</BoxStyled>
</Root>
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import * as React from 'react';
import { experimentalStyled as styled } from '@mui/material/styles';
import Box, { BoxProps as MuiBoxProps } from '@mui/material/Box';
import { experimentalStyled as styled, Theme } from '@mui/material/styles';
import { SxProps } from '@mui/system';
import Typography from '../components/Typography';

interface FormFeedbackProps extends MuiBoxProps {
interface FormFeedbackProps extends React.HTMLAttributes<HTMLDivElement> {
error?: boolean;
success?: boolean;
sx?: SxProps<Theme>;
}

const BoxStyled = styled(Box, {
const Root = styled('div', {
shouldForwardProp: (prop) => prop !== 'error' && prop !== 'success',
})<FormFeedbackProps>(({ theme, error, success }) => ({
padding: theme.spacing(2),
Expand All @@ -22,16 +23,12 @@ const BoxStyled = styled(Box, {
}),
}));

function FormFeedback(
props: React.HTMLAttributes<HTMLDivElement> & FormFeedbackProps,
) {
export default function FormFeedback(props: FormFeedbackProps) {
const { className, children, error, success, ...others } = props;

return (
<BoxStyled error={error} success={success} className={className} {...others}>
<Root error={error} success={success} className={className} {...others}>
<Typography color="inherit">{children}</Typography>
</BoxStyled>
</Root>
);
}

export default FormFeedback;
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const ProductHeroLayoutRoot = styled('section')(({ theme }) => ({
},
}));

const Background = styled(Box)({
const Background = styled('div')({
position: 'absolute',
left: 0,
right: 0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const ProductHeroLayoutRoot = styled('section')(({ theme }) => ({
},
}));

const Background = styled(Box)({
const Background = styled('div')({
position: 'absolute',
left: 0,
right: 0,
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-material-ui/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@ module.exports.rules = {
'no-hardcoded-labels': require('./rules/no-hardcoded-labels'),
'rules-of-use-theme-variants': require('./rules/rules-of-use-theme-variants'),
'no-empty-box': require('./rules/no-empty-box'),
'no-styled-box': require('./rules/no-styled-box'),
'straight-quotes': require('./rules/straight-quotes'),
};
55 changes: 55 additions & 0 deletions packages/eslint-plugin-material-ui/src/rules/no-styled-box.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copy from https://github.com/eslint/eslint/blob/95075251fb3ce35aaf7eadbd1d0a737106c13ec6/lib/rules/utils/ast-utils.js#L299
// Why is this not exported by ESLint?
/**
* Retrieve `ChainExpression#expression` value if the given node a `ChainExpression` node. Otherwise, pass through it.
* @param {ASTNode} node The node to address.
* @returns {ASTNode} The `ChainExpression#expression` value if the node is a `ChainExpression` node. Otherwise, the node.
*/
function skipChainExpression(node) {
return node && node.type === 'ChainExpression' ? node.expression : node;
}

/**
* @type {import('eslint').Rule.RuleModule}
*/
const rule = {
meta: {
docs: {
description: 'Disallow use of styled(Box), we prefer the sx prop over system props.',
},
messages: {
noBox: "The use of styled(Box) is not allowed, use styled('div') instead.",
},
type: 'suggestion',
fixable: 'code',
schema: [],
},
create(context) {
return {
CallExpression(node) {
const callee = skipChainExpression(node.callee);
if (callee.type !== 'Identifier') {
return;
}
if (callee.name !== 'styled') {
return;
}
if (!node.arguments[0]) {
return;
}

if (node.arguments[0].type === 'Identifier' && node.arguments[0].name === 'Box') {
context.report({
node,
messageId: 'noBox',
fix: (fixer) => {
return fixer.replaceText(node.arguments[0], "'div'");
},
});
}
},
};
},
};

module.exports = rule;
72 changes: 72 additions & 0 deletions packages/eslint-plugin-material-ui/src/rules/no-styled-box.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
const eslint = require('eslint');
const rule = require('./no-styled-box');

const ruleTester = new eslint.RuleTester({
parser: require.resolve('@typescript-eslint/parser'),
parserOptions: {
ecmaFeatures: { jsx: true },
},
});

ruleTester.run('no-styled-box', rule, {
valid: [
`
import { styled } from '@mui/system';
styled('div');
`,
`
import { styled } from '@mui/system';
styled('div', {});
`,
],
invalid: [
{
code: `
import { styled } from '@mui/system';
import Box from '@mui/material/Box';
const foo = styled(Box)({
color: 'red',
});
`,
errors: [
{
messageId: 'noBox',
type: 'CallExpression',
},
],
output: `
import { styled } from '@mui/system';
import Box from '@mui/material/Box';
const foo = styled('div')({
color: 'red',
});
`,
},
{
code: `
import { styled } from '@mui/system';
import Box from '@mui/material/Box';
const foo = styled(Box, {})({
color: 'red',
});
`,
errors: [
{
messageId: 'noBox',
type: 'CallExpression',
},
],
output: `
import { styled } from '@mui/system';
import Box from '@mui/material/Box';
const foo = styled('div', {})({
color: 'red',
});
`,
},
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import Box from '@mui/joy/Box';
/**
* styled API type check
*/
const StyledBox = styled(Box)(
const StyledBox = styled('div')(
({ theme }) => ({
padding: 32,
display: 'grid',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('colorInversionUtil', () => {

it('should not throw error using styled API', () => {
expect(() => {
styled(Box)(applySoftInversion('primary'), applySolidInversion('primary'));
styled('div')(applySoftInversion('primary'), applySolidInversion('primary'));
}).not.to.throw();
});
});
3 changes: 2 additions & 1 deletion packages/mui-system/src/Box/Box.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from 'react';
import { Box, Theme, styled } from '@mui/system';
import { Box, styled } from '@mui/system';

interface TestProps {
test?: string;
Expand Down Expand Up @@ -105,6 +105,7 @@ function TestFillPropCallback() {
/>;
}

// eslint-disable-next-line material-ui/no-styled-box
const StyledBox = styled(Box)`
color: white;
`;
Expand Down

0 comments on commit ea16771

Please sign in to comment.