-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[pickers] Rename AdapterDateFns
into AdapterDateFnsV2
and AdapterDateFnsV3
into AdapterDateFns
#16082
Conversation
@@ -50,17 +49,11 @@ describe('<AdapterDateFnsJalali />', () => { | |||
placeholder: 'YYYY/MM/DD hh:mm aa', | |||
value: '1397/02/25 09:35 ق.ظ.', | |||
}, | |||
faJalaliIR: { |
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.
Such a locale no longer exists on v4.
Co-authored-by: Michel Engelen <[email protected]> Signed-off-by: Lukas Tyla <[email protected]>
import transformFieldValue from '../rename-and-move-field-value-type'; | ||
|
||
import { JsCodeShiftAPI, JsCodeShiftFileInfo } from '../../../types'; | ||
|
||
export default function transformer(file: JsCodeShiftFileInfo, api: JsCodeShiftAPI, options: any) { | ||
file.source = transformAdapterDateFnsImports(file, api, options); |
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.
WDYT, should we include it in the safe preset if it is NOT idempotent?
Running it repeatedly will change the imports back and forth. 🙈
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.
WDYT, should we include it in the safe preset if it is NOT idempotent?
Oh crap, that's a good question.
Could it be work adding a small comment at the end of the line that we can check to make it idempotent? 😬
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.
I don't imagine how we could do it cleanly (without adding some marker comment)... 🤷
It's the nature of the rename, that we are essentially swapping the adapter names, with the exception that AdapterV3
becomes Adapter
and then after a subsequent run would become AdapterV2
at which point the codemod would no longer rename it back to Adapter
, but it's still:
`AdapterV3` -> run codemod -> `Adapter` -> run codemod -> `AdapterV2`
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.
I don't imagine how we could do it cleanly (without adding some marker comment)... 🤷
Me neither
The only solution I see is to transform:
import { AdapterDateFnsV3 } from '@mui/x-date-pickers/AdapterDateFnsV3';
into
// x-codemod/v8.0.0/pickers/adapter-date-fns-imports makes sure this codemod is idempotent. Don't remove unless you are sure not to run the v8.0.0 preset safe again.
import { AdapterDateFns } from '@mui/x-date-pickers/AdapterDateFns';
And check for the existence of the comment before transforming.
Which is far from great 😬
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.
Yeah, exactly... 🙈
Let's go the slightly risky route and wait for user feedback if this seems problematic. 🤔
@@ -1129,6 +1129,36 @@ If you were using them, you need to replace them with the following code: | |||
+ extends BaseMultiInputPickersTextFieldProps<true> {} | |||
``` | |||
|
|||
## ✅ Rename `date-fns` adapter imports | |||
|
|||
:::warning |
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.
Added a warning here to align with the codemod package readme.
Does it sound like a decent compromise @flaviendelangle @michelengelen? 🤔
We can always remove it from the preset-safe
and keep it opt-in if there are complaints... 🤷
``` | ||
|
||
::: | ||
> [!WARNING] |
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.
Fixes #14478.
AdapterDateFnsJalali
.