Skip to content

refactor: start using emotion for styling #9909

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

Merged
merged 17 commits into from
Sep 29, 2023
Merged

refactor: start using emotion for styling #9909

merged 17 commits into from
Sep 29, 2023

Conversation

aslilac
Copy link
Member

@aslilac aslilac commented Sep 28, 2023

Part of #9924

Progress 9_4

We're starting with 157 usages of makeStyles across 154 files. This changes brings that down to 143 usages across 140 files.

Initial impressions are good!

@aslilac
Copy link
Member Author

aslilac commented Sep 29, 2023

Notes so far:

  • Which?

    • import { type Theme, useTheme } from "@emotion/react";
    • import { type Theme, useTheme } from "@mui/system";
    • import { type Theme, useTheme } from "@mui/material/styles";
    • Probably a moot point because of css={(theme) => ...}
  • Perf concerns about css`` needing to parse CSS all the time

  • Perf concerns in general about recomposing styles on every render

    • Though I imagine that makeStyles wasn't any better, so probably a non-issue?
  • Overall, dev experience seems to be pretty nice/workable. Porting is going smoothly, and I've even done a couple tricky ones already!

@aslilac aslilac changed the title chore: emotional damage refactor: start using emotion for styling Sep 29, 2023
@aslilac aslilac marked this pull request as ready for review September 29, 2023 16:47
</ErrorBoundary>
</ThemeProvider>
<MuiThemeProvider theme={dark}>
<EmotionThemeProvider theme={dark}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need both? Or is it only for the transition?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, we can't get rid of MuiThemeProvider as long as we use MUI, and making Emotion aware of the theme has some nice DX upsides, so this is gonna persist for a while.

@aslilac aslilac merged commit 2b5428e into main Sep 29, 2023
@aslilac aslilac deleted the emotional-damage branch September 29, 2023 19:08
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2023
@aslilac aslilac linked an issue Sep 29, 2023 that may be closed by this pull request
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace makeStyles
2 participants