[codemod] Add `optimal-imports` for v5 #27404

mnajdova · 2021-07-22T11:55:43Z

Add optimal-imports codemod compatible with v5. It is included in the preset-safe codemod.

mui-pr-bot · 2021-07-22T11:57:38Z

Generated by 🚫 dangerJS against f6a89e7

mnajdova · 2021-07-22T12:01:39Z

packages/material-ui-codemod/src/v5.0.0/private-imports.test/actual.js

+import TableContext from '@material-ui/core/Table/TableContext';
+import TabScrollButton from '@material-ui/core/Tabs/TabScrollButton';
+import SwitchBase from '@material-ui/core/internal/SwitchBase';


Not sure if these should be ignored or not, but I've added them as they were there in the v4.0.0/optimal-imports. I've listed potential solutions to this in the PR description.

eps1lon

This looks good so far. Could use some test what happens if we already have an import from @material-ui/core/colors or what happens if we have two private imports for the same source.

And the sorting is also not tested. It's not clear to me whether we want to sort alphabetically (the code could be more explicit here instead of .sort()) or keep the imports as they appeared in the original source.

If people need them sorted, then they can use other lint plugins.

eps1lon · 2021-07-22T11:59:45Z

packages/material-ui-codemod/src/v5.0.0/private-imports.js

+  };
+
+  root.find(j.ImportDeclaration).forEach((path) => {
+    if (path.value.importKind && path.value.importKind !== 'value') return;


these are easy to miss.

Going to add a lint rule though that requires braces for single-statement ifs

oliviertassinari · 2021-07-22T12:16:06Z

None of these are exported from their parent directory, so the codemod basically breaks these imports

They have always been private, AFAIK, I would almost advocate for removing the imports, but what we currently do, by ignoring them sounds great too.

mnajdova · 2021-07-22T12:17:47Z

They have always been private, AFAIK, I would almost advocate for removing the imports, but what we currently do, by ignoring them sounds great too.

@oliviertassinari the codemod is currently NOT ignoring them, but I could make a list of the imports we want to still support and they can be ignored.

packages/material-ui-codemod/src/util/allowedPrivateImportsList.js

docs/src/pages/guides/migration-v4/migration-v4.md

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

mnajdova · 2021-07-23T12:35:37Z

@eps1lon I believe it is good to go now. I've basically just copied the optimal-imports and replaced /es/ with /modern/ in the paths.

eps1lon · 2021-07-26T09:23:46Z

@eps1lon I believe it is good to go now. I've basically just copied the optimal-imports and replaced /es/ with /modern/ in the paths.

It's still unclear from the PR description and name what this codemod is supposed to do. If this is just v4.0.0/optimal-imports then name it as such (v5.0.0/optimal-imports). But we don't want to touch private imports at all since we couldn't convert them anyway (otherwise they're not private).

mnajdova · 2021-07-26T09:30:03Z

It's still unclear from the PR description and name what this codemod is supposed to do. If this is just v4.0.0/optimal-imports then name it as such (v5.0.0/optimal-imports). But we don't want to touch private imports at all since we couldn't convert them anyway (otherwise they're not private).

We could rename it to optimal-imports, I've renamed it to private-imports because of #27321 (comment)

eps1lon · 2021-07-26T10:03:37Z

We could rename it to optimal-imports, I've renamed it to private-imports because of #27321 (comment)

I would ignore that statement since it is based on some false assumptions and statements that are missing context:

In v4, we have introduced a codemod https://github.com/mui-org/material-ui/blob/next/packages/material-ui-codemod/README.md#optimal-imports to migrate ALL these private import instances.

We do not migrate all: https://github.com/mui-org/material-ui/blob/4b128c53b895b863e5fd7c4069b53c1598203421/packages/material-ui-codemod/src/v4.0.0/optimal-imports.test/expected.js#L59-L61

optimal suggests that it's optional, it's not.

The same applies to the v4 version. The problem still stands that we can't codemod private imports. We could only remove these imports which would break the build (which makes the codemod not suitable for preset-safe). Instead, we should add a lint rule for path imports that has a fixer for those path imports that are available via public paths.

But that's probably out-of-scope. For now we just want the same implementation of optimal-imports so let's document it as such (PR descriptions are important). optimal-imports was not a good name, private-imports is just as much but these days "concerns" seem to matter more. If somebody feels strongly about their concerns we can go with either name since I don't think the naming matters.

The problem was that the PR description was incomplete so all I could go off was the name and the name was inaccurate.

siriwatknp

👍

mnajdova added 2 commits July 22, 2021 13:44

Add private-imports codemod

47d2b0e

Showcase failing private imports

1d2a7bf

mnajdova requested review from eps1lon, oliviertassinari and siriwatknp July 22, 2021 11:56

mnajdova mentioned this pull request Jul 22, 2021

[docs] Update color imports #27321

Merged

1 task

mnajdova commented Jul 22, 2021

View reviewed changes

eps1lon reviewed Jul 22, 2021

View reviewed changes

oliviertassinari removed their request for review July 22, 2021 12:16

Remove sorting, add more tests, remove private imports from test

a921ca1

github-actions bot mentioned this pull request Jul 22, 2021

Update snapshots for pr/27404 eps1lon/mui-scripts-incubator#2799

Closed

mnajdova added 2 commits July 22, 2021 15:04

include codemod in preset-safe, update migration guide, add more tests

c9c4c0b

Add allow list for the internal components

1c90446

mnajdova requested a review from eps1lon July 22, 2021 15:16

lint issue

8cfc78c

mnajdova commented Jul 22, 2021

View reviewed changes

packages/material-ui-codemod/src/util/allowedPrivateImportsList.js Outdated Show resolved Hide resolved

oliviertassinari reviewed Jul 22, 2021

View reviewed changes

docs/src/pages/guides/migration-v4/migration-v4.md Outdated Show resolved Hide resolved

Update docs/src/pages/guides/migration-v4/migration-v4.md

bcd0f38

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>

eps1lon changed the title ~~[codemod] Add private-imports codemode for v5~~ [codemod] Add private-imports for v5 Jul 23, 2021

mnajdova added 3 commits July 23, 2021 12:47

revert private-imports to behave exactly as optimal-imports

0fcf289

prettier

a7c35d1

trigger build

3a16144

This comment has been minimized.

Sign in to view

add modern in the paths

2a47680

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 24, 2021

Merge branch 'next' into feat/codemod-private-imports

a9e5e7e

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Jul 26, 2021

siriwatknp mentioned this pull request Jul 26, 2021

[codemod] Add jss to emotion codemod #27292

Merged

1 task

oliviertassinari added the v5.x migration label Jul 26, 2021

renamed to optimal-imports

3c2ddb0

mnajdova changed the title ~~[codemod] Add private-imports for v5~~ [codemod] Add optimal-imports for v5 Jul 26, 2021

eps1lon approved these changes Jul 26, 2021

View reviewed changes

siriwatknp approved these changes Jul 26, 2021

View reviewed changes

fix idempotent test

f6a89e7

mnajdova merged commit 423479b into mui:next Jul 26, 2021

siriwatknp mentioned this pull request Oct 4, 2021

[codemod] Fix optimal-imports to support v4 and v5-alpha, beta #28812

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[codemod] Add `optimal-imports` for v5 #27404

[codemod] Add `optimal-imports` for v5 #27404

mnajdova commented Jul 22, 2021 •

edited

mui-pr-bot commented Jul 22, 2021 •

edited

mnajdova Jul 22, 2021

eps1lon left a comment

eps1lon Jul 22, 2021

oliviertassinari commented Jul 22, 2021

mnajdova commented Jul 22, 2021 •

edited

This comment has been minimized.

mnajdova commented Jul 23, 2021

eps1lon commented Jul 26, 2021

mnajdova commented Jul 26, 2021

eps1lon commented Jul 26, 2021

siriwatknp left a comment

[codemod] Add optimal-imports for v5 #27404

[codemod] Add optimal-imports for v5 #27404

Conversation

mnajdova commented Jul 22, 2021 • edited

mui-pr-bot commented Jul 22, 2021 • edited

mnajdova Jul 22, 2021

Choose a reason for hiding this comment

eps1lon left a comment

Choose a reason for hiding this comment

eps1lon Jul 22, 2021

Choose a reason for hiding this comment

oliviertassinari commented Jul 22, 2021

mnajdova commented Jul 22, 2021 • edited

This comment has been minimized.

mnajdova commented Jul 23, 2021

eps1lon commented Jul 26, 2021

mnajdova commented Jul 26, 2021

eps1lon commented Jul 26, 2021

siriwatknp left a comment

Choose a reason for hiding this comment

[codemod] Add `optimal-imports` for v5 #27404

[codemod] Add `optimal-imports` for v5 #27404

mnajdova commented Jul 22, 2021 •

edited

mui-pr-bot commented Jul 22, 2021 •

edited

mnajdova commented Jul 22, 2021 •

edited