Skip to content

Commit

Permalink
[system] Fix unnecessary styles created from sx (mui#33752)
Browse files Browse the repository at this point in the history
  • Loading branch information
siriwatknp authored and Daniel Rabe committed Nov 29, 2022
1 parent 49b4f1c commit d67c4d3
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 7 deletions.
9 changes: 9 additions & 0 deletions packages/mui-styled-engine-sc/src/index.d.ts
Expand Up @@ -21,6 +21,15 @@ export { default as StyledEngineProvider } from './StyledEngineProvider';
export { default as GlobalStyles } from './GlobalStyles';
export * from './GlobalStyles';

/**
* For internal usage in `@mui/system` package
*/
// eslint-disable-next-line @typescript-eslint/naming-convention
export function internal_processStyles(
tag: React.ElementType,
processor: (styles: any) => any,
): void;

// These are the same as the ones in @mui/styled-engine
// CSS.PropertiesFallback are necessary so that we support spreading of the mixins. For example:
// '@font-face'?: Fontface | Fontface[]
Expand Down
12 changes: 12 additions & 0 deletions packages/mui-styled-engine-sc/src/index.js
Expand Up @@ -36,6 +36,18 @@ export default function styled(tag, options) {
return stylesFactory;
}

// eslint-disable-next-line @typescript-eslint/naming-convention
export const internal_processStyles = (tag, processor) => {
// Styled-components attaches an instance to `componentStyle`.
// https://github.com/styled-components/styled-components/blob/da8151762dcf72735ffba358173d4c097f6d5888/packages/styled-components/src/models/StyledComponent.ts#L257
//
// The instance contains `rules` (the styles)
// https://github.com/styled-components/styled-components/blob/da8151762dcf72735ffba358173d4c097f6d5888/packages/styled-components/src/models/ComponentStyle.ts#L23
if (tag.componentStyle) {
tag.componentStyle.rules = processor(tag.componentStyle.rules);
}
};

export { ThemeContext, keyframes, css } from 'styled-components';
export { default as StyledEngineProvider } from './StyledEngineProvider';
export { default as GlobalStyles } from './GlobalStyles';
9 changes: 9 additions & 0 deletions packages/mui-styled-engine/src/index.d.ts
Expand Up @@ -11,6 +11,15 @@ export { default as StyledEngineProvider } from './StyledEngineProvider';
export { default as GlobalStyles } from './GlobalStyles';
export * from './GlobalStyles';

/**
* For internal usage in `@mui/system` package
*/
// eslint-disable-next-line @typescript-eslint/naming-convention
export function internal_processStyles(
tag: React.ElementType,
processor: (styles: any) => any,
): void;

export interface SerializedStyles {
name: string;
styles: string;
Expand Down
10 changes: 10 additions & 0 deletions packages/mui-styled-engine/src/index.js
@@ -1,3 +1,4 @@
/* eslint-disable no-underscore-dangle */
import emStyled from '@emotion/styled';

export default function styled(tag, options) {
Expand Down Expand Up @@ -25,6 +26,15 @@ export default function styled(tag, options) {
return stylesFactory;
}

// eslint-disable-next-line @typescript-eslint/naming-convention
export const internal_processStyles = (tag, processor) => {
// Emotion attaches all the styles as `__emotion_styles`.
// Ref: https://github.com/emotion-js/emotion/blob/16d971d0da229596d6bcc39d282ba9753c9ee7cf/packages/styled/src/base.js#L186
if (Array.isArray(tag.__emotion_styles)) {
tag.__emotion_styles = processor(tag.__emotion_styles);
}
};

export { ThemeContext, keyframes, css } from '@emotion/react';
export { default as StyledEngineProvider } from './StyledEngineProvider';
export { default as GlobalStyles } from './GlobalStyles';
20 changes: 13 additions & 7 deletions packages/mui-system/src/createStyled.js
@@ -1,4 +1,5 @@
import styledEngineStyled from '@mui/styled-engine';
/* eslint-disable no-underscore-dangle */
import styledEngineStyled, { internal_processStyles as processStyles } from '@mui/styled-engine';
import { getDisplayName } from '@mui/utils';
import createTheme from './createTheme';
import propsToClassKey from './propsToClassKey';
Expand Down Expand Up @@ -82,7 +83,17 @@ export default function createStyled(input = {}) {
slotShouldForwardProp = shouldForwardProp,
styleFunctionSx = defaultStyleFunctionSx,
} = input;

const systemSx = (props) => {
const theme = isEmpty(props.theme) ? defaultTheme : props.theme;
return styleFunctionSx({ ...props, theme });
};
systemSx.__mui_systemSx = true;

return (tag, inputOptions = {}) => {
// Filter out the `sx` style function from the previous styled component to prevent unnecessary styles generated by the composite components.
processStyles(tag, (styles) => styles.filter((style) => !style?.__mui_systemSx));

const {
name: componentName,
slot: componentSlot,
Expand Down Expand Up @@ -131,7 +142,6 @@ export default function createStyled(input = {}) {
// On the server Emotion doesn't use React.forwardRef for creating components, so the created
// component stays as a function. This condition makes sure that we do not interpolate functions
// which are basically components used as a selectors.
// eslint-disable-next-line no-underscore-dangle
return typeof stylesArg === 'function' && stylesArg.__emotion_real !== stylesArg
? ({ theme: themeInput, ...other }) => {
return stylesArg({
Expand Down Expand Up @@ -176,10 +186,7 @@ export default function createStyled(input = {}) {
}

if (!skipSx) {
expressionsWithDefaultTheme.push((props) => {
const theme = isEmpty(props.theme) ? defaultTheme : props.theme;
return styleFunctionSx({ ...props, theme });
});
expressionsWithDefaultTheme.push(systemSx);
}

const numOfCustomFnsApplied = expressionsWithDefaultTheme.length - expressions.length;
Expand All @@ -194,7 +201,6 @@ export default function createStyled(input = {}) {
// On the server Emotion doesn't use React.forwardRef for creating components, so the created
// component stays as a function. This condition makes sure that we do not interpolate functions
// which are basically components used as a selectors.
// eslint-disable-next-line no-underscore-dangle
styleArg.__emotion_real !== styleArg
) {
// If the type is function, we need to define the default theme.
Expand Down
39 changes: 39 additions & 0 deletions packages/mui-system/src/createStyled.test.js
@@ -1,5 +1,6 @@
import * as React from 'react';
import { expect } from 'chai';
import { spy } from 'sinon';
import { ThemeProvider, createTheme } from '@mui/material/styles';
import { createRenderer } from 'test/utils';
import createStyled from './createStyled';
Expand Down Expand Up @@ -51,6 +52,44 @@ describe('createStyled', () => {
});
});

describe('composition', () => {
it('should call styleFunctionSx once', () => {
const styled = createStyled();
const spySx = spy();
const Child = styled('div')({});

render(<Child sx={spySx} />);

expect(spySx.callCount).to.equal(2); // React 18 renders twice in strict mode.
});

it('should still call styleFunctionSx once', () => {
const styled = createStyled();
const spySx = spy();
const Child = styled('div')({});
const Parent = styled(Child)({});

render(<Parent sx={spySx} />);

expect(spySx.callCount).to.equal(2); // React 18 renders twice in strict mode.
});

it('both child and parent still accept `sx` prop', () => {
const styled = createStyled();
const Child = styled('div')({});
const Parent = styled(Child)({});

const { container } = render(
<React.Fragment>
<Parent sx={{ color: 'rgb(0, 0, 255)' }} />
<Child sx={{ color: 'rgb(255, 0, 0)' }} />
</React.Fragment>,
);
expect(container.firstChild).toHaveComputedStyle({ color: 'rgb(0, 0, 255)' });
expect(container.lastChild).toHaveComputedStyle({ color: 'rgb(255, 0, 0)' });
});
});

describe('styles', () => {
it('styles of pseudo classes of variants are merged', () => {
const theme = createTheme({
Expand Down

0 comments on commit d67c4d3

Please sign in to comment.