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

[docs] Document bundle size #40549

Closed

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jan 11, 2024

See #40547 (comment)

Bundle size matters. I think it's important we document it in the docs. Documenting:

  • Forces us to measure it
  • Means developers know we know what it is
  • Means developers easily know if it's good enough for their use cases

https://deploy-preview-40549--material-ui.netlify.app/base-ui/react-autocomplete/

For example, see propeldata/ui-kit#157 for why, there is a reputation for bloat to debunk (Base UI) and solve (Material UI).


Ok, this PR solution it's not very clean, solving #40591 correctly would be a dream, but I think this is better than nothing. I would 💯 to add this to all the Base UI docs pages. I started to add it on a few more pages.

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation performance labels Jan 11, 2024
@michaldudak
Copy link
Member

To me, the issue you linked proves it doesn't so much - they chose Base UI despite having a large bundle size (although the numbers they show are for Material UI), because "the bundle size can be addressed with tree-shaking".
Still, displaying it in the docs won't do any harm.

@michaldudak michaldudak changed the title [docs] Document bundlesize [docs] Document bundle size Jan 11, 2024
Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Definitely think we should figure out a different way to showcase this — having a lonely bullet point with the bundle size before the Intro content looks random 😅

Screenshot 2024-01-12 at 08 39 54

Intuitvely, I'd add it to the ComponentLinkHeader chips on the header. Maybe there's a way to do it in this PR that's not as complex.

@samuelsycamore
Copy link
Contributor

Definitely think we should figure out a different way to showcase this — having a lonely bullet point with the bundle size before the Intro content looks random 😅
Intuitvely, I'd add it to the ComponentLinkHeader chips on the header.

This makes sense to me—no reason to add these links manually when it could be done programmatically.

oliviertassinari and others added 2 commits January 14, 2024 22:09
Co-authored-by: Michał Dudak <[email protected]>
Signed-off-by: Olivier Tassinari <[email protected]>
@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 15, 2024

Definitely think we should figure out a different way to showcase this — having a lonely bullet point with the bundle size before the Intro content looks random

I agree but I think that current PR is already much better than what we have on HEAD today.

It took 10 minutes to add. I have explored a bit the automation route, this is what I could come up with in 60 minutes: #40592. It will take probably 4 hours to get it to production (and then I expect the need for follow-up, about 4 more hours).

@samuelsycamore
Copy link
Contributor

samuelsycamore commented Jan 15, 2024

I don't really understand the point of adding this text to the page manually when we already have the "Bundle size" button right above it. It looks out of place, and redundant.

Screenshot 2024-01-15 at 10 30 04 AM

If we do want to add this info to each page, I think we should do so in a way that's coherent with the doc structure, so we don't need to change it again later. Maybe something like this?

## Introduction

{{ intro text }}
{{ intro demo }}

### Bundle size

{{ size and link }}

## Component

etc.

edit: I missed #40592 before but I think that solution is far preferable over adding text to the docs!

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 16, 2024

I don't really understand the point of adding this text to the page manually when we already have the "Bundle size" button right above it. It looks out of place, and redundant.

@samuelsycamore The problem is the effort it takes today to get the information, which can be a differentiator:

  1. If you know Material UI, and know its bundle size is bloated, land on the Base UI page, you might not even think twice about it, just close the tab (thinking its Material UI because the docs page looks close) and move to the next Google search hit because you know it's bloated
  2. If you know it's not bloated, and want to know the size, you have to go through this workflow, how much time is this? 10s-20s?
Screen.Recording.2024-01-16.at.02.21.01.mov

I think this solution looks much better than the text

So, how about we can combine the two PR: have the look and feel of #40592 and hard code the data of #40549.

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jan 16, 2024

Now, I think there is also a case to be made in favor of continuing in this direction of this PR. The aspiration is to bring back the marketing/selling section, for example of https://mui.com/material-ui/react-modal/:

we removed it in https://mui.com/base-ui/react-modal/ which feels like a step backward.


A benchmark:

https://www.radix-ui.com/primitives/docs/components/dialog

resonates with me (the approach, not the content). If we position the highlights relative to how it's the best component in its own space, then you are more likely to convince new users.

@danilo-leal
Copy link
Contributor

That makes sense to me; it's generally something that'd help folks quickly grasp what each component provides at a level. Though I definitely feel like it's a totally different thread than "documenting bundle size". I imagine you're bringing it up because they sit closely in the Radix docs design, but these are separate initiatives — would leave it out of this PR and discuss it separately in another one.

@oliviertassinari
Copy link
Member Author

Moving this to mui/base-ui#602

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants