[docs] Add custom default props handler #15473
Conversation
|
@material-ui/core: parsed: +0.03% , gzip: +0.52% Details of bundle changes.Comparing: 9634f29...1e321d2
|
eps1lon
force-pushed
the
docs/custom-default-props-resolver
branch
from
2a8f203
to
d13ace0
Compare
|
I think we should consider merging this. While the gzip increase is unfortunate we get at least a decrease in parsed size. It also helps apps using the If we push more components away from |
|
Im happy to merge this. I’ll have a look at some code before I officially approve it. To me one of the benefits of trimming down the library, by using functional components, means we can relax our reservations of increasing the bundle size a little. |
eps1lon
force-pushed
the
docs/custom-default-props-resolver
branch
from
d13ace0
to
6791d1a
Compare
| [classes[`wrap-xs-${String(wrap)}`]]: wrap != null, | ||
| [classes[`align-items-xs-${String(alignItems)}`]]: alignItems != null, | ||
| [classes[`align-content-xs-${String(alignContent)}`]]: alignContent != null, | ||
| [classes[`justify-xs-${String(justify)}`]]: justify != null, |
There was a problem hiding this comment.
Did you consider this strategy?
| [classes[`justify-xs-${String(justify)}`]]: justify != null, | |
| [classes[`justify-xs-${String(justify)}`]]: justify != 'flex-start', |
|
Does is close #15042? The only drawback I can think of: What about people accessing our |
oliviertassinari
force-pushed
the
docs/custom-default-props-resolver
branch
from
6791d1a
to
c636219
Compare
They were never part of our public API. We had one issue where someone was using them but it wasn't in a meaningful way. We have the theming for overriding default props. |
|
@eps1lon What do we miss to merge the pull request? |
Early commit to see bundle size impact.
Removes some default values that served no purpose
|
Was just waiting for a react-docgen release to use. Just need to resolve merge conflicts and it's good to go from my perspective. |
eps1lon
force-pushed
the
docs/custom-default-props-resolver
branch
from
1e321d2
to
a4b4742
Compare
eps1lon
force-pushed
the
docs/custom-default-props-resolver
branch
from
a4b4742
to
b06c83b
Compare
|
@material-ui/core: parsed: +0.03% , gzip: +0.63% Details of bundle changes.Comparing: 0bb185c...b06c83b
|
eps1lon
changed the title
|
@eps1lon I haven't dug into the code too much but do you think this is something that could be contributed back to |
|
@NMinhNguyen If I remember correctly then it was pretty rough to integrate this in the base function because they had some weird fallback functionality and accounted for all sorts of component patterns. It looked like the base function needed some refactoring first to account for todays understanding what components are. Not sure I want to get involved into that. You can copy the code from this implementation and use it in your own code base. We essentially just run two default props handlers: the default from react-docgen and a custom one that looks inside the function body: // parse.js
import { defaultHandlers, parse as docgenParse } from 'react-docgen';
// https://github.com/eps1lon/material-ui/blob/b06c83bea600c31ee1b61b8c7676f7f6065c05b6/docs/src/modules/utils/defaultPropsHandler.js
import muiDefaultPropsHandler from './defaultPropsHandler';
const reactAPI = docgenParse(src, resolvers, defaultHandlers.concat(muiDefaultPropsHandler));I'd suggest you open an issue in react-docgen with a feature request and link to this example implementation. |
Allows resolving default values for props inside the function body
This will come in handy when we also resolve default values from the theme via hooks (see #15023).
TODO
[ ] HiddenLeaving this for now. Adds quite a bit of bundle size for no benefit (currently)resolutionrelease