[system] Fix various issues reported by using @mui/styled-engine-sc #29035

mnajdova · 2021-10-13T15:10:05Z

Fixing some issues found by using @mui/styled-engine-sc directly in the @mui/system. Inspired by #29036. The changes from #29036 are also included here. The purpose is to battle test how well the integration with styled-components is really working.

Fixes #28905
Fixes #27167

List of problems found:

TS is broken - all styled-component's utils should be compatible with mui's styled() if @mui/styled-engine-sc is used
Issues found:
https://codesandbox.io/s/elegant-matan-k7mw4?file=/src/App.tsx We need to omit the ref from the components where props extends React.HTMLProps
Unstyled demos are broken - fixed by b7807f3 created [core] Fix issues when using styled-components #29048
CssBaseline is broken tests failing. False alarm, it was because the styles were tested with jsdom. Everything is fine when using browser, so I just disabled jsdom for them: 75e92d7
styled() tests are broken - https://app.circleci.com/pipelines/github/mui-org/material-ui/53651/workflows/20940c4d-7ec7-4477-b55b-363156535221/jobs/305430

The issues (not blocking) that I could not resolve are:

https://app.circleci.com/pipelines/github/mui-org/material-ui/53694/workflows/a7e393cb-2552-4d25-98d0-0a6db7d9b517/jobs/305731 it comes from emotion, it differs from styled-components. It is expected.
RTL fixtures are broken - fixed by 22afcab one is still broke - it is related to Nested selectors not processed styled-components/stylis-plugin-rtl#22. It was fixed by Fixed flipping of the nested rules and supports queries styled-components/stylis-plugin-rtl#21, but styled-components v5 is not compatible with the v2 of the plugin, so nothing we can do about it at this point.
https://app.circleci.com/pipelines/github/mui-org/material-ui/53690/workflows/0461ad90-3578-4004-9952-12b608f76798/jobs/305699 - we are not bundling styled-components so we are good. Related to Error: 'typeOf' is not exported by node_modules/react-is/index.js, imported by node_modules/styled-components/dist/styled-components.esm.js styled-components/styled-components#3256

mui-pr-bot · 2021-10-13T15:12:26Z

Details of bundle changes

Generated by 🚫 dangerJS against 1ae3b8b

…ng up things

…tyled-engine-sc/better-support-styled-components-ts

…ponents-ts

mnajdova · 2021-10-15T11:50:45Z

Latest commit using @mui/styled-engine-sc as a dependency of the @mui/system - 8bfb23b

The failing CI checks are because of the unresolved issues described in the PR description. There is nothing we can do about them at this moment, but the important bits are that those are not revelnt to the users of the packages.

mnajdova · 2021-10-15T12:18:50Z

test/regressions/fixtures/Tabs/RTLVerticalTabs.js

-        </Box>
-      </ThemeProvider>
-    </CacheProvider>
+    <StyleSheetManager stylisPlugins={[rtlPluginSc]}>


Adding the StyleSheetManager too so that we can support styled-components in the future too.

mnajdova · 2021-10-15T12:19:42Z

packages/mui-system/src/styled.test.js

@@ -537,10 +537,11 @@ describe('styled', () => {

      const classList = Array.from(container.firstChild.classList);
      const regExp = new RegExp(`.*-MuiComponent-slot$`);
+      const regExpSC = new RegExp(`MuiComponent-slot.*`);


emotion appends, but styled-components prepends the label.

mnajdova · 2021-10-15T12:20:47Z

packages/mui-system/src/createStyled.d.ts

-}
-
-export function shouldForwardProp(propName: PropertyKey): boolean;
+export type CreateMUIStyled<T extends object = DefaultTheme> = CreateMUIStyledStyledEngine<


This is now using the CreateMUIStyled coming from the styled engine.

mnajdova · 2021-10-15T12:21:35Z

packages/mui-styled-engine/src/index.d.ts

@@ -6,3 +10,170 @@ export { default as StyledEngineProvider } from './StyledEngineProvider';

 export { default as GlobalStyles } from './GlobalStyles';
 export * from './GlobalStyles';
+
+export interface SerializedStyles {


These are basically just copied from the packages/mui-system/createStyled.d.ts.

mnajdova · 2021-10-15T12:22:22Z

packages/mui-styled-engine-sc/src/index.d.ts

 export { default as GlobalStyles } from './GlobalStyles';
 export * from './GlobalStyles';
+
+// These are the same as the ones in @mui/styled-engine


These are the same as the ones in @mui/styled-engine just using styled-components.

mnajdova · 2021-10-15T12:25:58Z

docs/src/components/home/StoreTemplatesBanner.tsx

@@ -95,7 +95,7 @@ const StoreTemplateLink = React.forwardRef<

 const StoreTemplateImage = React.forwardRef<
  HTMLImageElement,
-  { brand: TemplateBrand } & JSX.IntrinsicElements['img']
+  { brand: TemplateBrand } & Omit<JSX.IntrinsicElements['img'], 'ref'>


The changes on the refs are related to this issue: https://codesandbox.io/s/elegant-matan-k7mw4?file=/src/App.tsx
See styled-components/styled-components#3191 (comment)

We can also use React.ComponentPropsWithoutRef<'img'> instead. I will update if we decide to go with this option.

We can also use React.ComponentPropsWithoutRef<'img'> instead. I will update if we decide to go with this option.

Sounds good to me.

michaldudak · 2021-10-18T15:01:30Z

packages/mui-lab/src/CalendarPickerSkeleton/CalendarPickerSkeleton.tsx

@@ -120,7 +121,7 @@ const monthMap = [
 function CalendarPickerSkeleton(props: CalendarPickerSkeletonProps) {
  const { className, ...other } = useThemeProps<
    Theme,
-    JSX.IntrinsicElements['div'],
+    Omit<JSX.IntrinsicElements['div'], 'ref'> & { ref?: React.Ref<HTMLDivElement> },


I'm curious, how is it different from JSX.IntrinsicElements['div']? I saw that JSX.IntrinsicElements['div'] has a ref of type string | Ref<T> - does the string makes a difference that breaks things here?

type Ref<T> = RefCallback<T> | RefObject<T> | null; type LegacyRef<T> = string | Ref<T>; // this is the ref type of JSX.IntrinsicElements

the LegacyRef is wider than Ref, that's why there is a type error. The other way to fix is to define LegacyRef in packages/mui-styled-engine/src/index.d.ts. Not sure if this is a good idea.

The other way to fix is to define LegacyRef in packages/mui-styled-engine/src/index.d.ts. Not sure if this is a good idea.

I would be scared to do this, mainly because then if you switch to mui's styled() to styled-components styled() it would break, but we would like those two to behave the same.

docs/src/pages/premium-themes/onepirate/modules/views/ProductHeroLayout.tsx

…ponents-ts

NikhilKale123 · 2022-03-03T10:59:59Z

Not able to integrate make style for v5.2 in typescript.

mnajdova · 2022-03-07T10:37:40Z

Not able to integrate make style for v5.2 in typescript.

@NikhilKale123 please open an issue with a reproduction and we can take a look

mnajdova added 5 commits October 13, 2021 11:32

[styled-engine] Extract types for styled()

4de5ab8

[styled-engine-sc] Fix mixins, styled options, normalize style args

ec0ec07

[mui-material] Export css & keyframes

446d5c2

Small fix

528aee6

REVERT ME: Test ci using @mui/styled-engine-sc

3f67a30

mnajdova marked this pull request as draft October 13, 2021 15:10

mnajdova mentioned this pull request Oct 13, 2021

[material] Export css & keyframes #29036

Closed

1 task

mnajdova added 7 commits October 14, 2021 12:41

Remove the warning in styled-components one vs array of args is messi…

7326ec8

…ng up things

[core] Fix issues when using styled-components

b7807f3

Merge branch 'feat/fix-some-styled-components-inconsistencies' into s…

506afe0

…tyled-engine-sc/better-support-styled-components-ts

Add support for styled-components in the RTL fixtures

22afcab

fix yarn:lint

71b3046

Merge branch 'feat/fix-some-styled-components-inconsistencies' into s…

09884de

…tyled-engine-sc/better-support-styled-components-ts

revert changes in styled-engine-sc/src.index.js

7cccb67

mnajdova added bug 🐛 Something doesn't work package: styled-engine-sc Specific to styled-components umbrella For grouping multiple issues to provide a holistic view labels Oct 14, 2021

mnajdova changed the title ~~[DO NOT REVIEW][styled-engine] Moves styled types to the styled engiene packages~~ [umbrella] Track issues related to @mui/styled-engine-sc Oct 14, 2021

mnajdova added 9 commits October 14, 2021 16:27

Fix styled test for styled-components

3d29127

CssBaseline test in browser only

75e92d7

Fixed TS issues

888712c

Remove @ts-ignore

563ef17

Fix correct backgroundClip

de38895

lint issues

599d515

Merge branch 'master' into styled-engine-sc/better-support-styled-com…

4875297

…ponents-ts

Revert some changes

fece712

Typings cleanup

8bfb23b

mnajdova changed the title ~~[umbrella] Track issues related to @mui/styled-engine-sc~~ [system] Fix various issues reported by using @mui/styled-engine-sc Oct 15, 2021

Use @mui/styled-engine in the system

54fe6e8

mnajdova marked this pull request as ready for review October 15, 2021 12:17

mnajdova requested review from siriwatknp and oliviertassinari October 15, 2021 12:17

mnajdova commented Oct 15, 2021

View reviewed changes

mnajdova requested review from a team and removed request for oliviertassinari and siriwatknp October 15, 2021 12:26

michaldudak reviewed Oct 18, 2021

View reviewed changes

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 18, 2021

siriwatknp reviewed Oct 19, 2021

View reviewed changes

docs/src/pages/premium-themes/onepirate/modules/views/ProductHeroLayout.tsx Outdated Show resolved Hide resolved

mnajdova mentioned this pull request Oct 20, 2021

[styled] CSS Animations (@keyframes) do not work with the built-in "styled" function #24851

Closed

2 tasks

siriwatknp approved these changes Oct 20, 2021

View reviewed changes

Merge branch 'master' into styled-engine-sc/better-support-styled-com…

1d9716f

…ponents-ts

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Oct 20, 2021

Replace function calls without args in styled with objects

1ae3b8b

mnajdova merged commit 209444d into mui:master Oct 21, 2021

mnajdova mentioned this pull request Oct 27, 2021

Small TypeScript error after upgrading from 5.0.4 to 5.0.5 #29307

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[system] Fix various issues reported by using @mui/styled-engine-sc #29035

[system] Fix various issues reported by using @mui/styled-engine-sc #29035

mnajdova commented Oct 13, 2021 •

edited

Loading

mui-pr-bot commented Oct 13, 2021 •

edited

Loading

mnajdova commented Oct 15, 2021 •

edited

Loading

mnajdova Oct 15, 2021

mnajdova Oct 15, 2021

mnajdova Oct 15, 2021

mnajdova Oct 15, 2021

mnajdova Oct 15, 2021

mnajdova Oct 15, 2021

siriwatknp Oct 19, 2021 •

edited

Loading

michaldudak Oct 18, 2021

siriwatknp Oct 19, 2021 •

edited

Loading

mnajdova Oct 20, 2021

NikhilKale123 commented Mar 3, 2022 •

edited

Loading

mnajdova commented Mar 7, 2022

[system] Fix various issues reported by using @mui/styled-engine-sc #29035

[system] Fix various issues reported by using @mui/styled-engine-sc #29035

Conversation

mnajdova commented Oct 13, 2021 • edited Loading

mui-pr-bot commented Oct 13, 2021 • edited Loading

mnajdova commented Oct 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Oct 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siriwatknp Oct 19, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NikhilKale123 commented Mar 3, 2022 • edited Loading

mnajdova commented Mar 7, 2022

mnajdova commented Oct 13, 2021 •

edited

Loading

mui-pr-bot commented Oct 13, 2021 •

edited

Loading

mnajdova commented Oct 15, 2021 •

edited

Loading

siriwatknp Oct 19, 2021 •

edited

Loading

siriwatknp Oct 19, 2021 •

edited

Loading

NikhilKale123 commented Mar 3, 2022 •

edited

Loading