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

Bundle size improvements #1246

Open
atomiks opened this issue Dec 30, 2024 · 3 comments
Open

Bundle size improvements #1246

atomiks opened this issue Dec 30, 2024 · 3 comments
Labels
core Infrastructure work going on behind the scenes performance

Comments

@atomiks
Copy link
Contributor

atomiks commented Dec 30, 2024

Inspecting the production bundle with create-vite-app using @base-ui-components/react reveals some areas of improvement:

  1. Extra prop-types logic is included in the final bundle: Even though .propTypes assignments are removed in prod, there's some extra code related to it when inspecting the bundle. We should remove prop-types entirely if possible in the final build, and only use it for documentation
  2. Minified errors: Context error messages aren’t minified for production, we can use a simple short/reusable message for production instead
  3. Duplicated logic between Floating UI and Base UI: Another version of mergeReactProps is used in useInteractions, so we should re-create useInteractions to use our own version instead of importing it. There is also duplicated logic between our internal Composite component and useListNavigation. There's also a duplicated version of useId
  4. Needless destructuring and re-passing of variables: we can pass the whole object in most cases, reducing unnecessary variable assignments. This is done for some components but not others
  5. Avoid unnecessary dependencies: importing only Dialog needlessly includes all anchor positioning logic since useFloating() brings it in. If FloatingFocusManager can accept floatingRootContext object, then that hook won’t need to be used. Importing only Tooltip needlessly includes the tabbable dependency and related focus guard logic since FloatingPortal brings it in. This matters less if you're using multiple components, however, since this logic will be brought in at some point anyway
@atomiks atomiks added the core Infrastructure work going on behind the scenes label Dec 30, 2024
@github-project-automation github-project-automation bot moved this to Backlog in Base UI Dec 30, 2024
@colmtuite colmtuite moved this from Backlog to In progress in Base UI Dec 30, 2024
@oliviertassinari
Copy link
Member

  1. Even though .propTypes assignments are removed in prod, there's some extra code related to it when inspecting the bundle.

@atomiks What logic is left in production?

We should remove prop-types entirely if possible in the final build, and only use it for documentation

Did you mean process.env.NODE_ENV? Recently, I found two cases using our APIs that frustrated be because they didn't correctly use the prop-types:

What I anticipate to be great DX is for us to have our own custom prop-types package: mui/material-ui#43138.

  1. Minified errors:

@Janpot recently iterated on this, helping a bit bring this in Base UI: mui/material-ui#43904.

  1. Duplicated logic between Floating UI and Base UI

👍 related to #454

  1. Needless destructuring and re-passing of variables

@romgrk has been doing a lot of changes around this in the data grid. It's a performance killer too.


If we want to use this issue as an umbrella, more ideas:

  1. We include tabbable for focus trapping, 2.4 KB gzipped but we might be able to skip it: [Modal] Improve the focus logic material-ui#14545.

  2. I would dream of [core] Add test_bundle_size_monitor back #201 so we don't regress. Showing the data on the docs https://base-ui.com/react/components/accordion, e.g. [docs] Document bundle size material-ui#40549 would be ++.

@atomiks
Copy link
Contributor Author

atomiks commented Jan 3, 2025

@oliviertassinari

  1. Here's some of the code. This isn't present with other packages that don't use prop-types:
Screenshot 2025-01-03 at 6 25 47 pm

Packages that solely rely on TypeScript are the standard in terms of consumption, and prop-types isn't typically expected to exist at all imo

  1. Thanks for pointing this out! @Janpot maybe it'd be good if you could set this up for Base UI, or I can explore adding it myself?

@Janpot
Copy link
Member

Janpot commented Jan 6, 2025

Thanks for pointing this out! @Janpot maybe it'd be good if you could set this up for Base UI, or I can explore adding it myself?

I won't have time in the short term. Should you explore it yourself, we seem to do something very similar to what react does. To get you up to speed, these are the locations of where to look for how this is implemented:

  • babel plugin to minify, lint or annotate error codes
  • script to extract the error codes. Run each time you add or change errors
  • verify in CI that user has kept error codes up to date
  • error codes are extracted into a json file in the docs
  • a page in the docs to translate code back to message. We link to this in the message
  • In code we annotate errors to minify with a /* minify-error */ comment (this is different from react, they minify each error, we still want to be selective for now).
  • We have a formatter in @mui/utils that takes a code and parameters and crafts an error message.

What likely will need to be done before we can integrate in other projects is a way to make the message configurable so you can host it on your own docs. I was thinking we could just make the module name configurable in the babel plugin. You then have full control on as to where your error formatting happens (e.g. @base-ui/utils/formatError) and how. I can look into this once I have some free time, but feel free to take a stab at it in the meantime.

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 performance
Projects
Status: In progress
Development

No branches or pull requests

3 participants