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] Remove unnecessary Box #9831

Merged
merged 1 commit into from
Aug 13, 2023

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jul 29, 2023

On the surface level, it's wasteful to render a Box to not use it (a small perf hit)

Now, in longer terms, we have a few opportunities, from the most short-term win to the most long-term:

  1. The change done here.
  2. We could have a warning that tells us when we use a Box without any prop, so we can't regress on 0.
  3. We could have a plugin that automatically transforms the Box into a div at built-time, so we get the performance win, while also not having to add/remove the Box import
  4. We could address [system] Support sx on all primitive elements material-ui#23220, the Box abstraction is annoying, all these </Box> closing tags tells you nothing. It would be much clearer if we could use the native element directly or the class name directly like in https://panda-css.com/docs/utilities/spacing#all-sides.

So, you could see step 0 as a step backward from step 2 referential, but it's a step forward compared to step 3 so I think we should move forward, be long-term oriented.

@oliviertassinari oliviertassinari added the core Infrastructure work going on behind the scenes label Jul 29, 2023
@oliviertassinari oliviertassinari changed the title [core] Remove unecessary Box [core] Remove unnecessary Box Jul 29, 2023
@mui-bot
Copy link

mui-bot commented Jul 29, 2023

Netlify deploy preview

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

Updated pages

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 308.8 481 372.9 378.82 65.17
Sort 100k rows ms 415.4 768.4 607 594.24 118.077
Select 100k rows ms 136.1 195.7 191.5 171.9 27.432
Deselect 100k rows ms 93.2 196.5 181.7 145.44 40.848

Generated by 🚫 dangerJS against 601e929

@oliviertassinari oliviertassinari merged commit a0024a6 into mui:master Aug 13, 2023
4 checks passed
@oliviertassinari oliviertassinari deleted the no-empty-Box branch August 13, 2023 19:39
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants