[system] Add color manipulators #26668
Conversation
|
Details of bundle changes (experimental) @material-ui/system: parsed: +7.32% , gzip: +7.12% |
To follow up on the discussion we had in #26490 (comment) with @eps1lon. This comment makes me realized that I wasn't clear on what I had in mind. I don't think that the size of the system package matter for the argument that Sebastian used: tree-shaking. Here the color utilities are tree-shakable. It wasn't the case with the default theme, it's a direct dependency of the Box/styled. Actually, maybe we should have a custom entry point to make sure the bundle size of Continuing with bundle size, we might want to change this path: The idea of the standalone color manipulator is interesting, we talked about it in #13039. I personally feel like we are already doing it with the current state of this PR 😁. |
|
|
||
| return recomposeColor(color); | ||
| } | ||
| export { |
There was a problem hiding this comment.
Developers can't import deep this file so we could remove it and reexport it from packages/material-ui/src/styles/index.js. Would it be simpler?
There was a problem hiding this comment.
I was’t sure after this one #26648 Looked like with not exporting those typings from the original file I introduced breaking changes. Or maybe the usage in the issue was not something we support?
There was a problem hiding this comment.
I think that it's actually great that we broke this nested import for this user. It has always been private, a good feedback loop for him to stop doing it. We could enforce the constraints on all the modules with #26254.
There was a problem hiding this comment.
Ok, that’s what I thought but wasn’t sure. I will then export these from index
There was a problem hiding this comment.
Done, I needed to update the imports from core, but apart from that looks good :)
| @@ -2,9 +2,9 @@ import * as React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import clsx from 'clsx'; | |||
| import { unstable_composeClasses as composeClasses } from '@material-ui/unstyled'; | |||
| import { alpha } from '@material-ui/system'; | |||
There was a problem hiding this comment.
I was wondering about doing
| import { alpha } from '@material-ui/system'; | |
| import { alpha } from '@material-ui/system/colorManipulator'; |
to help on the tree-shaking in dev mode, but since we likely include the whole system anyway in the core for a single component, I'm not sure we need to care.
There was a problem hiding this comment.
Not sure if it will have any impact.. The styled() util already depends on styleFunctionSx which basically composes the whole system, so not sure if it is worthed..
There was a problem hiding this comment.
I think if the system is not that big, I would rather use from root.
import { alpha } from '@material-ui/system' is more convenient.
There was a problem hiding this comment.
@siriwatknp I think that we could have a different policy for what we document and what we use internally: #24147.
Exporting the color utils from
@material-ui/system. These are not specific to MD, and are useful for building design system.The package grow is around
+7.32%. If we think this is too big, we can export them from a new package, something like@material-ui/color-utilsor something similar.Resonates a bit with #13039.