[pickers] Finish the migration to be compatible with PigmentCSS #13643
Conversation
flaviendelangle
added
the
component: pickers
|
Deploy preview: https://deploy-preview-13643--material-ui-x.netlify.app/ |
| import { SingleInputTimeRangeFieldProps } from './SingleInputTimeRangeField.types'; | ||
| import { useSingleInputTimeRangeField } from './useSingleInputTimeRangeField'; | ||
| import { FieldType } from '../models'; | ||
|
|
||
| const useThemeProps = createUseThemeProps('MuiSingleInputTimeRangeField'); |
There was a problem hiding this comment.
Nitpick: Do you know if it is mandatory to duplicate the name prop?
Is it used in static extraction or something else? 🤔
Currently, it seems a bit strange. 🙈
There was a problem hiding this comment.
Honestly I have no idea
I think I read it month ago. I'd say it's just that PigmentCSS will replace it be an import links it to the build-time theme based on the name that is in createUseThemeProps and ignore the one of useThemeProps and we kept the one in useThemeProps to avoid a breaking change.
| @@ -1,16 +1,19 @@ | |||
| import * as React from 'react'; | |||
| import clsx from 'clsx'; | |||
| import { styled, alpha, useThemeProps } from '@mui/material/styles'; | |||
| import { alpha } from '@mui/material/styles'; | |||
There was a problem hiding this comment.
Will keeping the alpha call outside of zero-styled allow for static extraction? 🤔
There was a problem hiding this comment.
I think so, but @brijeshb42 can confirm/infirm this one 👍
| @@ -2,13 +2,13 @@ import * as React from 'react'; | |||
| import PropTypes from 'prop-types'; | |||
| import clsx from 'clsx'; | |||
| import { useRtl } from '@mui/system/RtlProvider'; | |||
| import { styled, useThemeProps } from '@mui/material/styles'; | |||
There was a problem hiding this comment.
To keep the static-extraction intact we should avoid adding new imports from @mui/material/styles directly.
WDYT about creating an ESLint rule for that? 🤔
However, we'd have to think of what to do with type imports... Maybe import type could be skipped by an ESLint rule? 🤔
There was a problem hiding this comment.
@brijeshb42 is this something that is planned on the core side?
I guess it could benefit all MUI teams trying to migrate and to avoid re-using direct imports by mistake.
I don't think we can totally bloke @mui/material/styles since we want to use useTheme
There was a problem hiding this comment.
Makes sense. We'd need too many ignores for those. 🙈
I don't think we can totally bloke
@mui/material/stylessince we want to useuseTheme
Which ideally would be eventually replaced with the @mui/system import if there were no issues with theme merging/scoping. 🤔 🤞
There was a problem hiding this comment.
Yes, not sure when this would be though
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
removed
the
PR: out-of-date
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Closes #12277
This PR will remain opened until
@mui/materialreaches its v6 stable release and Pigment reaches its v1 stable release.By then we should have a better view of what we want to do.