[codemod] Add accordion props deprecation #40771

siriwatknp · 2024-01-24T10:05:19Z

transformation:

 <Accordion
-    TransitionComponent={CustomTransition}
-    TransitionProps={{ unmountOnExit: true }}
+    slots={{ transition: CustomTransition }}
+    slotProps={{ transition: { unmountOnExit: true } }}
 />

existing slots and slotProps are also handled.

Note

Adjusting the codemod structure a bit. It was really painful in v5 to navigate between codemod (the implementation and test cases are separated in different folder.

Moving forward, I think the implementation and test cases should be in the same folder. This is just a DX improvement for our team, there is no impact on consumers.

So from:

/src
  /version
    /transformation.test
      actual.js
      expected.js
    …a lot of folder
    transformation.js
    transformation.test.js

to:

/src
  /version
    /transformation
      /test-cases
        actual.js
        expected.js
      transformation
      transformation.test.js
      index.js
    …a lot of folder

I have followed (at least) the PR section of the contributing guide.

mui-bot · 2024-01-24T10:07:41Z

Netlify deploy preview

https://deploy-preview-40771--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 22fbadb

DiegoAndai

Thanks for working on this @siriwatknp!

The new file restructure makes sense to me.

@mnajdova, this is the way we envision deprecation codemods working: Deprecations might not be tied to a specific version, for example, the Accordion ones from this PR will exist in v5 and v6. The idea is to have a deprecations codemod that holds all current deprecations of the stable version.

Whenever we add a deprecation, we add the respective codemod to the deprecations codemods.
Whenever a deprecated API is removed and becomes a breaking change, that codemod is copied over to the version of the breaking change. For example, if TransitionComponent is removed in v7, the accordion-props codemod is copied to the v7 codemods. When v7 becomes the stable version, that codemod is removed from the deprecation codemods.

Users on the stable version would be able to run deprecations/all codemod any time they need to, as these should be idempotent. This way we can also start testing codemods as soon as possible, when the deprecation comes out.

The same goes for the migration guide.

What do you think?

mnajdova · 2024-01-24T11:08:18Z

Makes sene! 👍

DiegoAndai

Hey! I left a couple of comments 😊

Also a question: I tried running the codemod locally to modify the test cases (just to try it out), and it modifies tests-cases/actual.js as expected, but it doesn't modify test-cases/theme.actual.js. You know why that might be?

DiegoAndai · 2024-01-24T20:01:31Z

packages/mui-codemod/src/deprecations/all/deprecations-all.js

This file should be index.js, right? Or have an index file that reexports it. Otherwise, it won't be found here when running transform deprecations/all

DiegoAndai · 2024-01-24T20:25:20Z

packages/mui-codemod/src/deprecations/accordion-props/accordion-props.test.js

+        const actual = transform(
+          {
+            source: read('./test-cases/actual.js'),
+            path: require.resolve('./test-cases/actual.js'),


What's the difference between the source and the path properties? I couldn't find any reference to path inside the transform function.

DiegoAndai · 2024-01-24T22:16:33Z

Another question regarding codemods, the <path> provided is traversed recursively? I mean given

my-library/
├── my-components/
│   ├── my-accordion.tsx

If I run the codemod on the my-library path, would my-accordion.tsx be modified?

siriwatknp · 2024-01-25T03:33:01Z

Another question regarding codemods, the <path> provided is traversed recursively? I mean given
my-library/
├── my-components/
│   ├── my-accordion.tsx
If I run the codemod on the my-library path, would my-accordion.tsx be modified?

Yes, that's right.

DiegoAndai · 2024-01-25T18:54:11Z

I tried running the codemod locally to modify the test cases (just to try it out), and it modifies tests-cases/actual.js as expected, but it doesn't modify test-cases/theme.actual.js. You know why that might be?

I figured it out: the difference lies in the AST parsing. For some reason, when I run the codemod locally the TransitionComponent is considered an ObjectProperty instead of a Property (related issue). For some reason, it must be running with a different parser, babel, maybe. I think the correct type is Property, as far as I understand, jscodeshift should be using recast under the hood, and recast users Property, not ObjectProperty. Maybe it's a limitation of running the codemod locally, so we should merge as is (after fixing this) and see if it works when bundled.

siriwatknp · 2024-01-26T02:50:36Z

I tried running the codemod locally to modify the test cases (just to try it out), and it modifies tests-cases/actual.js as expected, but it doesn't modify test-cases/theme.actual.js. You know why that might be?

I figured it out: the difference lies in the AST parsing. For some reason, when I run the codemod locally the TransitionComponent is considered an ObjectProperty instead of a Property (related issue). For some reason, it must be running with a different parser, babel, maybe. I think the correct type is Property, as far as I understand, jscodeshift should be using recast under the hood, and recast users Property, not ObjectProperty. Maybe it's a limitation of running the codemod locally, so we should merge as is (after fixing this) and see if it works when bundled.

I figured out that the test is not correct, it should use @babel/parser (since we default the codemod parser to tsx) but instead rely on default recast. Anyway, fixed in a05e8a2.

siriwatknp · 2024-01-26T06:25:29Z

@DiegoAndai I think it's good to go. I added a CONTRIBUTING.md in this PR as well because I always forget about the steps and script when testing locally.

…mod/deprecations

DiegoAndai

Looks good, there's two test cases that seem not to work

DiegoAndai · 2024-01-26T18:59:16Z

packages/mui-codemod/CONTRIBUTING.md

Very useful!

DiegoAndai · 2024-01-26T19:11:09Z

packages/mui-codemod/src/util/findComponentJSX.js

+  // case 1: import ComponentName from '@mui/material/ComponentName';
+  // case 2: import { ComponentName } from '@mui/material';
+  // case 3: import { ComponentName as SomethingElse } from '@mui/material';


Nice that we cover these! Good catch

DiegoAndai · 2024-01-26T19:16:00Z

packages/mui-codemod/src/deprecations/accordion-props/test-cases/expected.js

+<Accordion slots={{ root: 'div' }} slotProps={{ root: { className: 'foo' } }} />;
+<MyAccordion slots={outerSlots} slotProps={outerSlotProps} />;


This looks like it's not working. The transition keys are not getting added.

Nice catch, fixed!

…mod/deprecations

DiegoAndai

Thanks for taking this 🙌🏼

…mod/deprecations

siriwatknp added 3 commits January 24, 2024 13:36

init deprecations codemod

7a85173

update readme

5c4c9a2

add accordion-props

39bf946

siriwatknp added component: accordion This is the name of the generic UI component, not the React module! package: codemod Specific to @mui/codemod labels Jan 24, 2024

siriwatknp requested review from mnajdova and DiegoAndai January 24, 2024 10:05

DiegoAndai reviewed Jan 24, 2024

View reviewed changes

siriwatknp added 5 commits January 26, 2024 09:52

add missing index file

78e70aa

fix jscodeshift import for test

a05e8a2

added contributing guide

0db96f0

fix logic

e54092c

update test cases

280ece1

siriwatknp requested a review from DiegoAndai January 26, 2024 06:24

siriwatknp added 2 commits January 26, 2024 20:23

Merge branch 'master' of https://github.com/mui/material-ui into code…

09b3225

…mod/deprecations

fix guide

46f1f12

DiegoAndai reviewed Jan 26, 2024

View reviewed changes

DiegoAndai mentioned this pull request Jan 26, 2024

[material-ui] Standardize slots pattern and composed classes #40417

Open

siriwatknp added 2 commits January 29, 2024 17:07

fix assignObject logic

8a3e4ba

update CONTRIBUTING

4655232

siriwatknp requested a review from DiegoAndai January 29, 2024 10:08

Merge branch 'master' of https://github.com/mui/material-ui into code…

f00b718

…mod/deprecations

trigger build

3410a5e

DiegoAndai approved these changes Jan 29, 2024

View reviewed changes

Merge branch 'master' of https://github.com/mui/material-ui into code…

22fbadb

…mod/deprecations

siriwatknp enabled auto-merge (squash) January 30, 2024 03:40

siriwatknp merged commit c479c9b into mui:master Jan 30, 2024
18 checks passed

mostafa-rio pushed a commit to mostafa-rio/material-ui that referenced this pull request Feb 3, 2024

[codemod] Add accordion props deprecation (mui#40771)

079397c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[codemod] Add accordion props deprecation #40771

[codemod] Add accordion props deprecation #40771

siriwatknp commented Jan 24, 2024 •

edited

Loading

mui-bot commented Jan 24, 2024 •

edited

Loading

DiegoAndai left a comment •

edited

Loading

mnajdova commented Jan 24, 2024

DiegoAndai left a comment

DiegoAndai Jan 24, 2024

DiegoAndai Jan 24, 2024

DiegoAndai commented Jan 24, 2024

siriwatknp commented Jan 25, 2024

DiegoAndai commented Jan 25, 2024

siriwatknp commented Jan 26, 2024 •

edited

Loading

siriwatknp commented Jan 26, 2024

DiegoAndai left a comment

DiegoAndai Jan 26, 2024

DiegoAndai Jan 26, 2024

DiegoAndai Jan 26, 2024

siriwatknp Jan 29, 2024

DiegoAndai left a comment

		<Accordion slots={{ root: 'div' }} slotProps={{ root: { className: 'foo' } }} />;
		<MyAccordion slots={outerSlots} slotProps={outerSlotProps} />;

[codemod] Add accordion props deprecation #40771

[codemod] Add accordion props deprecation #40771

Conversation

siriwatknp commented Jan 24, 2024 • edited Loading

Note

mui-bot commented Jan 24, 2024 • edited Loading

Netlify deploy preview

Bundle size report

DiegoAndai left a comment • edited Loading

Choose a reason for hiding this comment

mnajdova commented Jan 24, 2024

DiegoAndai left a comment

Choose a reason for hiding this comment

DiegoAndai Jan 24, 2024

Choose a reason for hiding this comment

DiegoAndai Jan 24, 2024

Choose a reason for hiding this comment

DiegoAndai commented Jan 24, 2024

siriwatknp commented Jan 25, 2024

DiegoAndai commented Jan 25, 2024

siriwatknp commented Jan 26, 2024 • edited Loading

siriwatknp commented Jan 26, 2024

DiegoAndai left a comment

Choose a reason for hiding this comment

DiegoAndai Jan 26, 2024

Choose a reason for hiding this comment

DiegoAndai Jan 26, 2024

Choose a reason for hiding this comment

DiegoAndai Jan 26, 2024

Choose a reason for hiding this comment

siriwatknp Jan 29, 2024

Choose a reason for hiding this comment

DiegoAndai left a comment

Choose a reason for hiding this comment

siriwatknp commented Jan 24, 2024 •

edited

Loading

mui-bot commented Jan 24, 2024 •

edited

Loading

DiegoAndai left a comment •

edited

Loading

siriwatknp commented Jan 26, 2024 •

edited

Loading