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

[charts] Remove require condition from package.json exports #10272

Merged
merged 3 commits into from
Sep 8, 2023

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Sep 8, 2023

At the moment we don't have a working commonjs export for @mui/x-charts. We're using this alpha package to test the waters in the community about support for ESM modules. As such it makes sense to remove the non-working require condition, and supply a default condition as a fallback. Alternatively we could opt for removing the import condition altogether and just keeping default.

Closes #10257
Closes #10195

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Sep 8, 2023
@mui-bot
Copy link

mui-bot commented Sep 8, 2023

Netlify deploy preview

Netlify deploy preview: https://deploy-preview-10272--material-ui-x.netlify.app/

Updated pages

No updates.

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms -217.3 24.8 -217.3 -85 87.995
Sort 100k rows ms 577.4 1,483.1 1,419 1,173.44 337.393
Select 100k rows ms 604.1 899.4 770.2 748.52 98.5
Deselect 100k rows ms 169.5 310.3 174.6 206.12 53.357

Generated by 🚫 dangerJS against 0f3dde6

@Janpot Janpot changed the title [WIP] charts package.json exports [charts] Remove require condition from package.json exports Sep 8, 2023
@Janpot Janpot marked this pull request as ready for review September 8, 2023 11:15
@Janpot Janpot requested a review from LukasTy September 8, 2023 11:16
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

LGTM! 💯
Thank you for taking the time to investigate the problem. 🙏

@LukasTy LukasTy added component: charts This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work labels Sep 8, 2023
@LukasTy LukasTy merged commit 4e2ee49 into mui:master Sep 8, 2023
6 checks passed
@Janpot Janpot deleted the experiment-charts-esm branch September 19, 2023 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: charts This is the name of the generic UI component, not the React module! core Infrastructure work going on behind the scenes
Projects
None yet
3 participants