Skip to content

[Table] Migrate TableCell to emotion#24663

Merged
oliviertassinari merged 11 commits intomui:nextfrom
natac13:table-cell-emotion
Feb 12, 2021
Merged

[Table] Migrate TableCell to emotion#24663
oliviertassinari merged 11 commits intomui:nextfrom
natac13:table-cell-emotion

Conversation

@natac13
Copy link
Contributor

@natac13 natac13 commented Jan 28, 2021

#24405

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 28, 2021 •
edited
Loading

@material-ui/core: parsed: +0.21% , gzip: +0.14%

Details of bundle changes

Generated by 🚫 dangerJS against 05b37df

@mnajdova
Copy link
Member

mnajdova commented Jan 28, 2021

See #24658 (comment) for fixing the test error

@oliviertassinari oliviertassinari added the scope: table Changes related to the table. label Jan 28, 2021
@oliviertassinari oliviertassinari mentioned this pull request Jan 28, 2021
Closed
@natac13 natac13 mentioned this pull request Jan 28, 2021
Merged
1 task
@natac13 natac13 force-pushed the table-cell-emotion branch from 29f46e1 to 7f509aa Compare January 30, 2021 15:08
@oliviertassinari oliviertassinari self-assigned this Feb 12, 2021
@oliviertassinari oliviertassinari mentioned this pull request Feb 12, 2021
Merged
@github-actions github-actions bot mentioned this pull request Feb 12, 2021
Closed
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 12, 2021 •
edited
Loading

Regarding performance.

v4
Capture d’écran 2021-02-12 à 23 25 54

https://material-ui.com/performance/table-mui/

v5
Capture d’écran 2021-02-12 à 23 24 50

https://deploy-preview-24663--material-ui.netlify.app/performance/table-mui/

We had identified in the past, how moving from withStyles to makeStyle would yield a +10% in performance. So the new approach is close to x2 slower. On paper, it still has the potential to be faster:

Capture d’écran 2021-02-12 à 23 30 42

https://deploy-preview-24663--material-ui.netlify.app/performance/table-emotion/

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 12, 2021

If it really becomes an issue, we can spend time investigating workarounds, the TableCell is not a component like the other. For instance, we could consider a HTML primitive version like https://react-bootstrap.github.io/components/table/.

@oliviertassinari oliviertassinari merged commit 8f6b38a into mui:next Feb 12, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 12, 2021 •
edited
Loading

I get different perf results in the benchmark: #24892. The difference between v4 and v5 is smaller than I thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@oliviertassinari oliviertassinari

Labels

scope: table Changes related to the table.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@natac13 @mui-pr-bot @mnajdova @oliviertassinari