Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] perf: sx #44254

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
cb1539b
perf: initial work
romgrk Oct 29, 2024
be47607
perf: more work
romgrk Oct 29, 2024
e387422
lint
romgrk Oct 29, 2024
2da8c80
lint
romgrk Oct 29, 2024
5cf7676
lint
romgrk Oct 29, 2024
f65132c
lint
romgrk Oct 29, 2024
490e808
lint
romgrk Oct 29, 2024
17eb786
lint
romgrk Oct 29, 2024
6d2f759
lint
romgrk Oct 30, 2024
4f7de94
lint
romgrk Oct 30, 2024
89d3e11
perf: more work
romgrk Oct 30, 2024
d88d4db
lint
romgrk Oct 30, 2024
c288aac
perf: remove allocations
romgrk Oct 30, 2024
7d70f97
lint
romgrk Oct 30, 2024
a221c46
perf: container queries
romgrk Oct 30, 2024
b1cdfcb
lint
romgrk Oct 30, 2024
d4141ca
Merge branch 'master' into perf-sx
romgrk Nov 1, 2024
f7fe570
lint
romgrk Nov 1, 2024
ec67cbb
lint
romgrk Nov 1, 2024
5ef7075
perf: compose()
romgrk Nov 1, 2024
0123e02
refactor: alternateProp logic for fontFamilyCode
romgrk Nov 1, 2024
07e4410
lint
romgrk Nov 1, 2024
f202bc8
lint
romgrk Nov 1, 2024
d9f7017
lint
romgrk Nov 1, 2024
046d66b
lint
romgrk Nov 1, 2024
d586a77
perf: improve breakpoint mq cloning
romgrk Nov 1, 2024
8330b43
lint
romgrk Nov 1, 2024
b9642a7
fix: breakpoints
romgrk Nov 1, 2024
520a5d2
lint
romgrk Nov 1, 2024
096c50b
lint
romgrk Nov 1, 2024
f3749aa
lint
romgrk Nov 1, 2024
3171351
lint
romgrk Nov 1, 2024
7e0e0c1
lint
romgrk Nov 1, 2024
fff7e0b
perf: spacing
romgrk Nov 1, 2024
40cc09d
lint
romgrk Nov 1, 2024
c1fb261
fix: tests
romgrk Nov 1, 2024
739f020
lint
romgrk Nov 1, 2024
2d09be5
refactor: remove EMPTY_BREAKPOINTS
romgrk Nov 1, 2024
2874595
fix
romgrk Nov 1, 2024
e3de9d8
lint
romgrk Nov 1, 2024
6eac84f
lint
romgrk Nov 1, 2024
8790800
lint
romgrk Nov 1, 2024
9d995d0
lint
romgrk Nov 1, 2024
013e87d
lint
romgrk Nov 1, 2024
ec1914a
lint
romgrk Nov 1, 2024
6c32c64
refactor: hasBreakpoint
romgrk Nov 1, 2024
e6f2319
lint
romgrk Nov 1, 2024
db32450
lint
romgrk Nov 2, 2024
b120356
lint
romgrk Nov 2, 2024
aa57a19
refactor: removeUnusedBreakpoints
romgrk Nov 2, 2024
0b6c30d
refactor: iterateBreakpoints
romgrk Nov 2, 2024
811d63a
refactor: sortContainerQueries
romgrk Nov 2, 2024
419cfc6
lint
romgrk Nov 4, 2024
f096175
lint
romgrk Nov 4, 2024
3455a04
lint
romgrk Nov 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion packages/mui-system/src/breakpoints/breakpoints.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CSSObject } from '@mui/styled-engine';
import { Breakpoints } from '../createBreakpoints/createBreakpoints';
import type { Breakpoint } from '../createTheme';
import type { Breakpoint, Theme } from '../createTheme';
import { ResponsiveStyleValue } from '../styleFunctionSx';
import { StyleFunction } from '../Box';

Expand All @@ -15,6 +15,18 @@ export function resolveBreakpointValues<T>(

export function mergeBreakpointsInOrder(breakpoints: Breakpoints, styles: CSSObject[]): CSSObject;

export function buildBreakpoints(
target: any,
theme: Theme,
propValue: any,
styleFromPropValue: (
target: any,
key: string | undefined,
value: any,
breakpoint?: Breakpoint,
) => any,
): any;

export function handleBreakpoints<Props>(
props: Props,
propValue: any,
Expand Down
114 changes: 73 additions & 41 deletions packages/mui-system/src/breakpoints/breakpoints.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import PropTypes from 'prop-types';
import isObjectEmpty from '@mui/utils/isObjectEmpty';
import fastDeepAssign from '@mui/utils/fastDeepAssign';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
import fastDeepAssign from '@mui/utils/fastDeepAssign';
import deepAssign from '@mui/utils/deepAssign';

Copy link
Contributor Author

@romgrk romgrk Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I name something fast... it usually means there's a trade-off with correctness to focus on being "fast". Here it's that it doesn't handle exotic objects as other implementations will, as in, it will also traverse enumerable properties on the object's prototype. In the cases where we use this function (to create React props objects copies, or to create CSS style objects) we can safely assume that invariant is true. I should add this to the jsdoc, but I like having some sort of warning in the name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this to the jsdom would definitely help. On the other hand, I would recommend maybe moving this util outside of @mui/utils, if it is only used in the mui-system, to avoid developers accidently using it in other places where it may not be the right thing. If you already plan to use it in other places, then probably we can keep it in @mui/utils.

import deepmerge from '@mui/utils/deepmerge';
import merge from '../merge';
import { isCqShorthand, getContainerQuery } from '../cssContainerQueries';
Expand Down Expand Up @@ -34,48 +36,78 @@ const defaultContainerQueries = {
}),
};

export function handleBreakpoints(props, propValue, styleFromPropValue) {
const theme = props.theme || {};
const EMPTY_BREAKPOINTS = {
keys: [],
up: (key) => `@media (min-width:${values[key]}px)`,
};

function buildBreakpoint(target, key, value, initialKey, callback) {
target[key] ??= {};
callback(target, key, value, initialKey);
}

export function buildBreakpoints(target, theme, propValue, styleFromPropValue) {
theme ??= {};

if (Array.isArray(propValue)) {
const themeBreakpoints = theme.breakpoints || defaultBreakpoints;
return propValue.reduce((acc, item, index) => {
acc[themeBreakpoints.up(themeBreakpoints.keys[index])] = styleFromPropValue(propValue[index]);
return acc;
}, {});
const breakpoints = theme.breakpoints || defaultBreakpoints;
for (let i = 0; i < propValue.length; i += 1) {
buildBreakpoint(
target,
breakpoints.up(breakpoints.keys[i]),
propValue[i],
undefined,
styleFromPropValue,
);
}
return target;
}

if (typeof propValue === 'object') {
const themeBreakpoints = theme.breakpoints || defaultBreakpoints;
return Object.keys(propValue).reduce((acc, breakpoint) => {
if (isCqShorthand(themeBreakpoints.keys, breakpoint)) {
const breakpoints = theme.breakpoints || defaultBreakpoints;
const breakpointValues = breakpoints.values ?? values;

for (const key in propValue) {
if (isCqShorthand(breakpoints.keys, key)) {
const containerKey = getContainerQuery(
theme.containerQueries ? theme : defaultContainerQueries,
breakpoint,
key,
);
if (containerKey) {
acc[containerKey] = styleFromPropValue(propValue[breakpoint], breakpoint);
buildBreakpoint(target, containerKey, propValue[key], key, styleFromPropValue);
}
}
// key is breakpoint
else if (Object.keys(themeBreakpoints.values || values).includes(breakpoint)) {
const mediaKey = themeBreakpoints.up(breakpoint);
acc[mediaKey] = styleFromPropValue(propValue[breakpoint], breakpoint);
// key is key
else if (key in breakpointValues) {
const mediaKey = breakpoints.up(key);
buildBreakpoint(target, mediaKey, propValue[key], key, styleFromPropValue);
} else {
const cssKey = breakpoint;
acc[cssKey] = propValue[cssKey];
const cssKey = key;
target[cssKey] = propValue[cssKey];
}
return acc;
}, {});
}

return target;
}

const output = styleFromPropValue(propValue);
styleFromPropValue(target, undefined, propValue);

return output;
return target;
}

function breakpoints(styleFunction) {
// false positive
export function handleBreakpoints(props, propValue, styleFromPropValue) {
const result = {};
return buildBreakpoints(result, props.theme, propValue, (target, key, value, initialKey) => {
const finalValue = styleFromPropValue(value, initialKey);
if (key) {
target[key] = finalValue;
} else {
fastDeepAssign(target, finalValue);
}
});
}

function setupBreakpoints(styleFunction) {
// eslint-disable-next-line react/function-component-definition
const newStyleFunction = (props) => {
const theme = props.theme || {};
Expand Down Expand Up @@ -110,28 +142,28 @@ function breakpoints(styleFunction) {
return newStyleFunction;
}

export function createEmptyBreakpointObject(breakpointsInput = {}) {
const breakpointsInOrder = breakpointsInput.keys?.reduce((acc, key) => {
const breakpointStyleKey = breakpointsInput.up(key);
acc[breakpointStyleKey] = {};
return acc;
}, {});
return breakpointsInOrder || {};
export function createEmptyBreakpointObject(breakpoints = EMPTY_BREAKPOINTS) {
const result = {};
for (let i = 0; i < breakpoints.keys.length; i += 1) {
result[breakpoints.up(breakpoints.keys[i])] = {};
}
return result;
}

export function removeUnusedBreakpoints(breakpointKeys, style) {
return breakpointKeys.reduce((acc, key) => {
const breakpointOutput = acc[key];
const isBreakpointUnused = !breakpointOutput || Object.keys(breakpointOutput).length === 0;
if (isBreakpointUnused) {
delete acc[key];
for (let i = 0; i < breakpointKeys.length; i += 1) {
const key = breakpointKeys[i];

if (isObjectEmpty(style[key])) {
delete style[key];
}
return acc;
}, style);
}

return style;
}

export function mergeBreakpointsInOrder(breakpointsInput, ...styles) {
const emptyBreakpoints = createEmptyBreakpointObject(breakpointsInput);
export function mergeBreakpointsInOrder(breakpoints, ...styles) {
const emptyBreakpoints = createEmptyBreakpointObject(breakpoints);
const mergedOutput = [emptyBreakpoints, ...styles].reduce(
(prev, next) => deepmerge(prev, next),
{},
Expand Down Expand Up @@ -197,4 +229,4 @@ export function resolveBreakpointValues({
}, {});
}

export default breakpoints;
export default setupBreakpoints;
9 changes: 1 addition & 8 deletions packages/mui-system/src/createStyled/createStyled.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import styledEngineStyled, { internal_mutateStyles as mutateStyles } from '@mui/styled-engine';
import isObjectEmpty from '@mui/utils/isObjectEmpty';
import { isPlainObject } from '@mui/utils/deepmerge';
import capitalize from '@mui/utils/capitalize';
import getDisplayName from '@mui/utils/getDisplayName';
Expand Down Expand Up @@ -281,14 +282,6 @@ function generateStyledLabel(componentName, componentSlot) {
return label;
}

function isObjectEmpty(object) {
// eslint-disable-next-line
for (const _ in object) {
return false;
}
return true;
}

// https://github.com/emotion-js/emotion/blob/26ded6109fcd8ca9875cc2ce4564fee678a3f3c5/packages/styled/src/utils.js#L40
function isStringTag(tag) {
return (
Expand Down
12 changes: 5 additions & 7 deletions packages/mui-system/src/merge/merge.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import deepmerge from '@mui/utils/deepmerge';

function merge(acc, item) {
if (!item) {
return acc;
}
const options = {
clone: false,
};

return deepmerge(acc, item, {
clone: false, // No need to clone deep, it's way faster.
});
function merge(acc, item) {
return deepmerge(acc, item, options);
}

export default merge;
27 changes: 0 additions & 27 deletions packages/mui-system/src/style/style.d.ts

This file was deleted.

95 changes: 0 additions & 95 deletions packages/mui-system/src/style/style.js

This file was deleted.

Loading