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] Fix @mui/material package building #35324

Merged
merged 1 commit into from Dec 12, 2022

Conversation

timbset
Copy link
Contributor

@timbset timbset commented Dec 2, 2022

Hi there.

I found out that material package built folder structure has changed unexpectedly since 5.10.3. I have researched and found that there were no reasons for that kind of change and the problem is in adding a new file to the package root in this PR: #32735 that affects this part of the building script. While learning changes I did not found any reason why this should stay as it is, I event did not find any usage of this file. So my idea is to remove files from the root by putting it into a folder, so the building algorithm is going to perform as it was before 5.10.3.

Why is the folder structure important to me? Well, I try to use best practices of using material-ui and one of them is to import specific component by a specific path instead of importing the whole package. Unfortunately, this approach does not work with ESM that is necessary for me. That's why I've reported the issue. Until it is fixed, I have to resolve all imports to a specific files in the package. I know that you do not promise that internal files are not going to be changed, but I also hope that all your changes are reasonable. The current one is not.

By the way, I don't pretend to be right about fixing the issue this way. So my main goal is to rollback a building algorithm.

@mui-bot
Copy link

mui-bot commented Dec 2, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-35324--material-ui.netlify.app/

@material-ui/core: parsed: -16.44% 😍, gzip: -10.10% 😍
@mui/material-next: parsed: -6.48% 😍, gzip: -4.44% 😍

Details of bundle changes

Generated by 🚫 dangerJS against c9c6872

@zannager zannager added package: material-ui Specific to @mui/material core Infrastructure work going on behind the scenes labels Dec 2, 2022
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.

Thanks for the detailed description on the PR. The goal of #32735 was to provide augmentation for simplifying some types to allow better TypeScript checks. The reason why it was added as a root was so that the import for the file would be consistent with other imports. Now the import needs to look like:

import "@mui/material/types/OverridableComponentAugmentation";

but honestly I am ok with it. We probably want to leave a comment in #19113 with the change once this gets merged.

@michaldudak michaldudak changed the title Fix @mui/material package building [core] Fix @mui/material package building Dec 12, 2022
@michaldudak michaldudak merged commit 0a4a997 into mui:master Dec 12, 2022
@timbset timbset deleted the fix/material-package-building branch December 12, 2022 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants