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

[system] Fix inconsistent multiple types for mergeBreakpointsInOrder #38405

Closed
2 tasks done
dylnic opened this issue Aug 10, 2023 · 5 comments · Fixed by #38749
Closed
2 tasks done

[system] Fix inconsistent multiple types for mergeBreakpointsInOrder #38405

dylnic opened this issue Aug 10, 2023 · 5 comments · Fixed by #38749
Assignees
Labels
bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. package: system Specific to @mui/system typescript

Comments

@dylnic
Copy link

dylnic commented Aug 10, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/mui-mergebreakpointsinorder-type-83gd2y

Steps:

  1. import { mergeBreakpointsInOrder } from "@mui/system";
  2. pass in a theme object created with createTheme();
  3. observe TypeScript error

Current behavior 😯

The typing for the mergeBreakpointsInOrder changes depending on where you import it from. If you import from the barrel file it appears to use an old type incompatible with the current theme breakpoints. However the function is typed correctly if you use a path import.

This works:
import { mergeBreakpointsInOrder } from "@mui/system/breakpoints";

This doesn't:
import { mergeBreakpointsInOrder } from "@mui/system";

Expected behavior 🤔

The same type should be used regardless of how you import the function.

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.
@dylnic dylnic added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Aug 10, 2023
@zannager zannager added the package: system Specific to @mui/system label Aug 10, 2023
@dylnic
Copy link
Author

dylnic commented Aug 10, 2023

Hi @mnajdova, I'm happy to raise a PR for this, let me know :)

@ZeeshanTamboli
Copy link
Member

@dylnic Thank you for bringing up this issue. You're absolutely right – the type definition should remain consistent, no matter where it's imported from. Please feel free to create a pull request to fix this. Additionally, would you be willing to share your specific use case for using this method?

@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Aug 30, 2023
@ZeeshanTamboli ZeeshanTamboli added the good first issue Great for first contributions. Enable to learn the contribution process. label Aug 30, 2023
@ZeeshanTamboli ZeeshanTamboli changed the title Multiple types for mergeBreakpointsInOrder [system] Fix inconsistent multiple types for mergeBreakpointsInOrder Aug 30, 2023
@imevanc
Copy link
Contributor

imevanc commented Aug 30, 2023

@dylnic Thank you for bringing up this issue. You're absolutely right – the type definition should remain consistent, no matter where it's imported from. Please feel free to create a pull request to fix this. Additionally, would you be willing to share your specific use case for using this method?

@ZeeshanTamboli - Can you assign this to me?

@dylnic
Copy link
Author

dylnic commented Aug 31, 2023

@ZeeshanTamboli no worries :) We're leveraging mui in a component library and need to add responsive props to some of the custom components that don't use mui under the hood.

@imevanc
Copy link
Contributor

imevanc commented Aug 31, 2023

@ZeeshanTamboli - Can you please add the system label or similar, so it doesn't break the pipeline? Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work good first issue Great for first contributions. Enable to learn the contribution process. package: system Specific to @mui/system typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants