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] Fix typos #1447

Merged
merged 2 commits into from
Apr 26, 2021
Merged

[docs] Fix typos #1447

merged 2 commits into from
Apr 26, 2021

Conversation

ZeeshanTamboli
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli commented Apr 20, 2021

Few typos in the docs.

@ZeeshanTamboli ZeeshanTamboli changed the title [docs] fix typos [docs] Fix typos Apr 20, 2021
@@ -66,7 +66,7 @@ We provide three options:

- Built with and exclusively for React ⚛️
- High performance 🚀
- Lightweight; less than [30 kB](https://bundlephobia.com/result?p=@material-ui/data-grid) gzipped with as few dependencies as possible.
- Lightweight; less than [55 kB](https://bundlephobia.com/result?p=@material-ui/data-grid) gzipped with as few dependencies as possible.
Copy link
Member

@oliviertassinari oliviertassinari Apr 20, 2021

Choose a reason for hiding this comment

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

This is not lightweight, this is almost bloated. I couldn't spot any major bump. It seems to be simply related to the features. Maybe we write them in a way that is particularly bundled size intensive. No idea, a plugin system become more relevant now (#924). We have to be careful. Soon nobody will want to use the MIT version because the bundle size tradeoff won't worth it. Also a good reason to port back the bundle size tracker #37.

Suggested change
- Lightweight; less than [55 kB](https://bundlephobia.com/result?p=@material-ui/data-grid) gzipped with as few dependencies as possible.
- Weight [55 kB](https://bundlephobia.com/result?p=@material-ui/data-grid) gzipped with as few dependencies as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we altogether remove this mentioning of 55kb bundle size from Features if it's risky to mention which may lead to less/no users. Better than mentioning false size (user doesn't know we forgot to update doc). When we act on #924 we can specify it as a good feature of isolation.
Also is data-grid tree-shakeable?

Copy link
Member

@oliviertassinari oliviertassinari Apr 20, 2021

Choose a reason for hiding this comment

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

Also is data-grid tree-shakeable

Yes, to some extent. For instance, because we didn't bundle the Toolbar, it's tree-shakeable. I think that ideally with #924, we would create a public hook API developers can use, with a plugin system.

Why not remove the weight 👍

Suggested change
- Lightweight; less than [55 kB](https://bundlephobia.com/result?p=@material-ui/data-grid) gzipped with as few dependencies as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

Copy link
Member

Choose a reason for hiding this comment

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

#924 is part of the beta so we need to start looking at it. I'm interested in picking it up.

Copy link
Member

@oliviertassinari oliviertassinari Apr 22, 2021

Choose a reason for hiding this comment

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

Actually no, I was wrong. It looks like tree-shaking doesn't work with our packages:

Screenshot 2021-04-22 at 20 00 11

This looks like a quick win. We have been careless about this aspect. https://github.com/mui-org/material-ui/blob/4d7fb7c197f6bc345e9705d45dfb5e03bef70075/packages/material-ui/package.json#L78

The previous investigation regarding bundle size opportunities: #954.

@DanailH It depends on what we want to optimize for. If bundle size is our biggest interest, I think that we should start with bundle size monitoring at each PR. Being able to continuously measure changes is paramount. We can only improve what you can measure. Same reason why writing tests is critical when solving complex problems.

The main value I see with #924 is that it makes us competitive with react-table. The great side nice effects being:

  1. more tree-shaking
  2. isolation of MIT vs. commercial
  3. allow community plugins
  4. force us to better architecture the state

Copy link
Member

Choose a reason for hiding this comment

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

I spend half an hour diving into why this happens. There are more issues than I thought. I have updated #18 with the problems and solutions. For instance, all the developers bundle all the locales 😱.

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Apr 23, 2021
@oliviertassinari
Copy link
Member

@mui-org/x Anyone wants to merge this one?

@dtassone dtassone merged commit 54b88b7 into mui:master Apr 26, 2021
@ZeeshanTamboli ZeeshanTamboli deleted the docs/fix-typos branch April 26, 2021 12:19
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants