Skip to content

[system] Add empty interfaces to fix issue with typescript module augmentation #43873

Merged
mnajdova merged 5 commits intomui:masterfrom
yonatan0:create-theme-interfaces
Oct 3, 2024
Merged

[system] Add empty interfaces to fix issue with typescript module augmentation #43873
mnajdova merged 5 commits intomui:masterfrom
yonatan0:create-theme-interfaces

Conversation

@yonatan0
Copy link
Contributor

@yonatan0 yonatan0 commented Sep 24, 2024

Fixes #43855

@mui-bot
Copy link

mui-bot commented Sep 24, 2024

Netlify deploy preview

https://deploy-preview-43873--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 1b2fe45

@Janpot Janpot added scope: system The system, the design tokens / styling foundations used across components. eg. @mui/system with MUI type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. labels Sep 24, 2024
@Janpot Janpot changed the title [mui/system] add empty interfaces to fix issue with typescript module augmentation [system] Add empty interfaces to fix issue with typescript module augmentation Sep 24, 2024
typography?: unknown;
mixins?: Mixins;
typography?: Typography;
zIndex?: unknown;
Copy link
Member

Choose a reason for hiding this comment

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

The only unknown left is zIndex. Is there a reason not to type it the same way as ThemeOptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my bad, fixed 🙏

@yonatan0
Copy link
Contributor Author

yonatan0 commented Sep 25, 2024

  1. Changed the zIndex type from Record<string, number> to a more generic interface: The original type was causing conflicts with @mui/material's createTheme function.
  2. Renamed zIndex to ZIndex: to comply with the linting rules that enforce PascalCase naming conventions.

@yonatan0
Copy link
Contributor Author

hey @Janpot,
i tried to figure out why the argos ci step failed, but with no success, maybe you have some insights for me on how to fix this?

Copy link
Member

@Janpot Janpot left a comment

Choose a reason for hiding this comment

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

This seems good to me

maybe you have some insights for me on how to fix this?

This screenshot is flaky, we haven't fully figured out why yet. You can safely ignore

@Janpot Janpot requested a review from mnajdova September 25, 2024 14:51
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

It's a great first conutrion on MUI 👌 People will appreciate the fix! :)

@mnajdova mnajdova merged commit e430fcc into mui:master Oct 3, 2024
@yonatan0 yonatan0 deleted the create-theme-interfaces branch October 3, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: system The system, the design tokens / styling foundations used across components. eg. @mui/system with MUI type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[mui/system] theme - typescript module augmentation

4 participants