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.

Sign up for GitHub

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

Jump to bottom

[utils] Fix "useId" & "useSyncExternalStore" imports to not be statically analyzable #43360

Merged
merged 6 commits into from
Sep 17, 2024
Merged

[utils] Fix "useId" & "useSyncExternalStore" imports to not be statically analyzable #43360

merged 6 commits into from
Sep 17, 2024

Conversation

yash49
Copy link
Contributor

@yash49 yash49 commented Aug 18, 2024

fixes #41190

@yash49 yash49 marked this pull request as draft August 18, 2024 12:31
@mui-bot
Copy link

mui-bot commented Aug 18, 2024 •
edited
Loading

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against b3840af

@yash49 yash49 changed the title [base-ui][material-ui] Fix "useId" & "useSyncExternalStore" import error after updating esbuild [base-ui][material-ui] Make "useId" & "useSyncExternalStore" imports statically unanalysable Aug 18, 2024
@yash49 yash49 changed the title [base-ui][material-ui] Make "useId" & "useSyncExternalStore" imports statically unanalysable [base-ui][mui-system] Make "useId" & "useSyncExternalStore" imports statically unanalysable Aug 18, 2024
@yash49 yash49 marked this pull request as ready for review August 18, 2024 12:50
@yash49 yash49 mentioned this pull request Aug 18, 2024
Closed
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Aug 18, 2024
@oliviertassinari oliviertassinari changed the title [base-ui][mui-system] Make "useId" & "useSyncExternalStore" imports statically unanalysable [base-ui][mui-system] Fix "useId" & "useSyncExternalStore" imports to not be statically analysable Aug 18, 2024
@zannager zannager requested a review from atomiks August 19, 2024 14:31
@yash49
Copy link
Contributor Author

yash49 commented Aug 26, 2024

hi @atomiks, can you please review this tiny pr?

atomiks
atomiks approved these changes Aug 27, 2024
View reviewed changes
Copy link
Contributor

@atomiks atomiks left a comment

Choose a reason for hiding this comment

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

I also switched from toString() to the {...React} technique in https://github.com/floating-ui/floating-ui and it has worked without issue.

@yash49
Copy link
Contributor Author

yash49 commented Aug 27, 2024

Thank you @atomiks for reviewing the PR.

@oliviertassinari / @zannager can we merge this PR ?

@oliviertassinari oliviertassinari mentioned this pull request Aug 27, 2024
Closed
@oliviertassinari oliviertassinari force-pushed the bug/make-imports-statically-unanalysable branch from 1a38207 to f28f154 Compare September 14, 2024 20:31
@oliviertassinari oliviertassinari changed the title [base-ui][mui-system] Fix "useId" & "useSyncExternalStore" imports to not be statically analysable [utils] Fix "useId" & "useSyncExternalStore" imports to not be statically analysable Sep 14, 2024
@oliviertassinari oliviertassinari added the package: utils Specific to the @mui/utils package label Sep 14, 2024
@oliviertassinari oliviertassinari enabled auto-merge (squash) September 14, 2024 20:37
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 14, 2024 •
edited
Loading

I did some clean-up. I think it's ready to be merged?

Looking at the useId inspired me to open mui/base-ui#616.

@siriwatknp siriwatknp merged commit 1275f37 into mui:master Sep 17, 2024
19 checks passed
@oliviertassinari oliviertassinari changed the title [utils] Fix "useId" & "useSyncExternalStore" imports to not be statically analysable [utils] Fix "useId" & "useSyncExternalStore" imports to not be statically analyzable Sep 17, 2024
@duy0000 duy0000 mentioned this pull request Oct 25, 2024
Open
@DMPatod DMPatod mentioned this pull request Oct 25, 2024
Open
@DMPatod DMPatod mentioned this pull request Oct 25, 2024
Open
@bansikah22 bansikah22 mentioned this pull request Oct 26, 2024
Merged
@Eskedar21 Eskedar21 mentioned this pull request Oct 26, 2024
Open
@josetutis josetutis mentioned this pull request Oct 26, 2024
Open
@Hoang-Nguyen-Huy Hoang-Nguyen-Huy mentioned this pull request Oct 26, 2024
Merged
This was referenced Oct 29, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atomiks atomiks atomiks approved these changes

@Janpot Janpot Janpot approved these changes

Assignees
No one assigned
Labels
bug 🐛 Something doesn't work package: utils Specific to the @mui/utils package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[base-ui] "useId" import error after updating esbuild
6 participants
@yash49 @mui-bot @oliviertassinari @Janpot @atomiks @siriwatknp