-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[zero] Add README with installation and basic usage #40761
Conversation
brijeshb42
commented
Jan 23, 2024
- I have followed (at least) the PR section of the contributing guide.
Netlify deploy previewhttps://deploy-preview-40761--material-ui.netlify.app/ Bundle size report |
3370d53
to
a7e7752
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very exciting stuff!
@@ -0,0 +1,238 @@ | |||
# zero-runtime | |||
|
|||
A zero-runtime CSS-in-JS library that extracts the colocated css to it's own css files at build-time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A zero-runtime CSS-in-JS library that extracts the colocated css to it's own css files at build-time. | |
A zero-runtime CSS-in-JS library that extracts colocated CSS to separate CSS files at build time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this sound like that each css definition will have its own css file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wasn't very clear to me exactly what was intended by the original sentence. Is there another way to phrase this? Maybe it requires more details?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, it depends on the route. So for ex, for next.js, each route might have its own css file. So all styled calls' CSS will be part of that route's css file.
Maybe just - that extracts the colocated CSS to the output CSS files
38c417e
to
6f4396f
Compare
6f4396f
to
d2b8bbb
Compare
const { experimental_extendTheme: extendTheme } = require('@mui/material/styles'); | ||
const theme = extendTheme(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we don't mention @mui/material/styles
and use a plain theme object?
const { experimental_extendTheme: extendTheme } = require('@mui/material/styles'); | |
const theme = extendTheme(); | |
const theme = { | |
colorSchemes: { | |
light: { | |
palette: { | |
brand: '...', | |
} | |
}, | |
dark: { | |
palette: { | |
brand: '...', | |
} | |
} | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We right now depend on the extendTheme
implementation to then call generateCssVars
in the zero-runtime
util's generateCss
. If we have to support plain objects, we can either implement the same in zero-runtime directly or we can move the theme implementation to @mui/theming
package that will only have all theme specific logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. I think we need to revisit the theming for zero-runtime. In my opinion, zero-runtime should work without theme config.
If developer tries to use theme, we throw the error/warning and show them where to add the theme.
Next, the theme object can be a plain object that they can control. Meaning, if I provide:
const theme = {
typography: {
h1: { … }, …
}
}
I should be able to use it from the theme callback
Then the CSS variables, we should consider as an opt-in feature. Meaning, developers have to follow our structure and pass in generateCssVars
to the theme.
I can take care of this task if you agree with the above model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, yes. It should work even with plain js objects. This was the case actually previously before we agreed on relying on making extendTheme
the default. We can still provide css variable token object even if they are not using extendTheme
. It would be possible if we extract out the css vars related utilities to either @mui/system
or independent package.
b0bf4b3
to
535c448
Compare
535c448
to
c2ce6bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍