Skip to content
New issue

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

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

Already on GitHub? # to your account

[core] Add exports field to packages #43521

Closed
wants to merge 29 commits into from

Conversation

DiegoAndai
Copy link
Member

Follow up on #41596 (comment). Draft to create CI builds to test with.

CC: @Janpot

@DiegoAndai DiegoAndai added the core Infrastructure work going on behind the scenes label Aug 29, 2024
@DiegoAndai DiegoAndai self-assigned this Aug 29, 2024
@DiegoAndai DiegoAndai marked this pull request as draft August 29, 2024 20:29
@mui-bot
Copy link

mui-bot commented Aug 29, 2024

Netlify deploy preview

Bundle size report

Bundle size will be reported once CircleCI build #742065 finishes.

Generated by 🚫 dangerJS against 659a479

@DiegoAndai
Copy link
Member Author

@Janpot I'm testing #41596 (comment). Initially the issue doesn't seem to be fixed by removing the internal package.json files.

I'm doing the same test we did a few months back:

Interestingly, back then, esmExternals: false fixed the issue, but now it doesn't, so progress I guess 😂

I'll do a debugging session, but I'm letting you know in case you want to test it on your side in the meantime.

@Janpot
Copy link
Member

Janpot commented Aug 30, 2024

@DiegoAndai I'v been working on this over the past week in

I've extracted some of the issues that popped up as they can be released separately:

I followed your steps to reproduce with the following dependencies:

"@mui/icons-material": "https://pkg.csb.dev/mui/material-ui/commit/d4953d45/@mui/icons-material",
"@mui/material": "https://pkg.csb.dev/mui/material-ui/commit/d4953d45/@mui/material",
"@mui/material-nextjs": "https://pkg.csb.dev/mui/material-ui/commit/d4953d45/@mui/material-nextjs",

And both pnpm dev and pnpm build work, regardless of the value of esmExternals. Am I missing something? We should probably check what's the difference between both our PRs, e.g. I think I have a different method of dealing with next/document.

I'm still in the process of testing how X works but that looks promising so far. I'm also going to verify the vite issues Yep, that works now.

The one unknown I have left is whether I can remove esmExternals from our docs. Initial tests suggest no, but have to dig deeper into it.

@DiegoAndai
Copy link
Member Author

Great work @Janpot 🙌🏼

When I read #43264, I thought we needed this PR to test it, but let's keep #43264 as the ongoing initiative.

I tested with #43264 and it works, so I'm closing this PR.

@DiegoAndai DiegoAndai closed this Aug 30, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants