mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.52k stars 32.19k forks source link

[docs] Problems with running codemod #23610

Closed phaux closed 3 years ago

phaux commented 3 years ago

I spent about 3 hours trying to upgrade my project from v4 to v5 and then I gave up.

There are all the issues I encountered:

First thing I tried was literally what was written in the README:

npm install -D @material-ui/codemod
npx jscodeshift -t box-sx-prop -d src/

It didn't work. Jscodeshift returned some cryptic error.

It took me a while to realize that the codemod path should be the actual path to the file in the package in node_modules. I also realized that the codemod is not there, so I tried installing again with @next tag and I finally made some progress:

npm install -D @material-ui/codemod@next
npx jscodeshift -d -t node_modules/@material-ui/codemod/lib/node/v5.0.0/box-sx-prop.js src/

It ran without errors, but it detected 0 files.

After some more digging in docs I figured out I have to specify extensions like that:

npx jscodeshift -d --extensions js,ts,jsx,tsx -t node_modules/@material-ui/codemod/lib/node/v5.0.0/box-sx-prop.js src/

Then it would error again with unexpected token errors.

After some searching on stackoverflow I found out you have to have a babel config in your project. I didn't have one, because I was using snowpack. I also tried installing parcel instead of snowpack because I know that it uses babel for compiling, but it didn't help.

I copy-pasted some config which I found on stackoverflow that supposedly should make it work:

// babel.config.js
module.exports = function (api) {
  api.cache(true)
  return {
    presets: ["@babel/preset-react", "@babel/preset-typescript"],
  }
}

That gave me errors about that the plugins aren't installed, even though both snowpack and parcel are installed and they work.

Okay, I installed the plugins manually as dev dependencies...

It still didn't work. I got the same unexpected token errors as before.

I give up. Funny thing is that if I changed all the Box props to sx manually then I would be already finished. It's not easy though, because they only produce a warning at runtime. TypeScript considers them not an error.

The app does work with alpha version. It only shows warnings in the console about the sx prop.

Current Behavior 😯

As above.

Expected Behavior πŸ€”

A perfect solution would be a command like npx jscodeshift @material-ui/codemod/box-sx-prop src/ which supports TypeScript out of the box.

Another solution would be to write better documentation. Ideally a list of commands which do all the necessary stuff and then uninstall all the unnecessary things.

Steps to Reproduce πŸ•Ή

As above.

Context πŸ”¦

I was trying to upgrade my MUI app which uses v4 to v5.

Your Environment 🌎

Tech Version
Material-UI 5.0.0-alpha.16
React 17.0.1
TypeScript 4.0.5
Babel 7.12.x
eps1lon commented 3 years ago

Thanks for the detailed walk-through!

jscodeshift with TypeScript isn't that obvious to use so we should probably add documentation for it. I guess the biggest problems are projects not using babel since they need to install and configure it first just for a codemod.

Ideally we'd link to jscodeshift docs since this isn't a problem with the codemod itself but the runner for it (jscodeshift).

oliviertassinari commented 3 years ago

What if we change the usage instructions from:

npm install -D @material-ui/codemod
npx jscodeshift -t <codemod-script> <path>
find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/lib/v5.0.0/box-sx-prop.js

to:

npx @material-ui/codemod src --codemod 'v5.0.0/box-sx-prop'

or to apply all the codemods under a prefix:

npx @material-ui/codemod src --codemod 'v5.0.0'

We could take care of a glob logic, providing the right arguments for TypeScript, maybe even solving the issue with Babel. I have seen something similar in https://github.com/thumbtack/thumbprint/tree/master/packages/thumbprint-codemods. They provide a slim wrapper on top of jscodeshift to make the nominal use case as simple as possible.

eps1lon commented 3 years ago

If you want to implement your own codemodding engine then be my guest. It will introduce even more problems though so not sure where you want to get the time for maintaining it.

oliviertassinari commented 3 years ago

@eps1lon What do you mean by codemodding engine? I should have precised that the benchmark is a small wrapper on top of jscodeshift to cover the nominal use case.

mkarajohn commented 3 years ago

I too wasted time trying to run a codemod from the ones provided, on a TS project, with no success. In my case I was not trying to upgrade to v5. @phaux Do you remember which stack overflow question you found? Googling for this issue proved more difficult than expected

phaux commented 3 years ago

@mkarajohn I don't remember, sorry.

BTW the issue doesn't affect me anymore. The Box props don't have to be passed as an object in sx prop so I just upgraded to latest alpha and resolved the rest of problems manually. Never had to run the codemod. Nice!

StuffByLiang commented 3 years ago

@eps1lon Would love to make a pr adding some documentation to use codemod with typescript! I'm migrating to mui v4 -> v5 and I also had a bit of a struggle getting mui's codemod to run with typescript. Thoughts?

Also for me, I used

npm install -D @material-ui/codemod@next
npx jscodeshift -t ./node_modules/@material-ui/codemod/node/v5.0.0/textfield-variant-prop.js ./src --extensions tsx --parser tsx

would preferably add it here https://github.com/mui-org/material-ui/tree/HEAD/packages/material-ui-codemod#box-sx-prop

eps1lon commented 3 years ago

Would love to make a pr adding some documentation to use codemod with typescript!

Sounds great. Feel free to go ahead and then we work with the initial proposal.

lionpunk commented 3 years ago

@StuffByLiang Your solution works fine

oliviertassinari commented 3 years ago

I would propose the following diff:

diff --git a/packages/material-ui-codemod/README.md b/packages/material-ui-codemod/README.md
index 6185fa264c..54ab06cfd6 100644
--- a/packages/material-ui-codemod/README.md
+++ b/packages/material-ui-codemod/README.md
@@ -11,10 +11,13 @@ APIs.

 ## Setup & Run

-- `npm install -D @material-ui/codemod`
+<!-- #default-branch-switch -->
+
+- `npm install -D @material-ui/codemod@next`
 - `npx jscodeshift -t <codemod-script> <path>`
 - Use the `-d` option for a dry-run and use `-p` to print the output
   for comparison
+- Use the `--extensions tsx --parser tsx` options to convert TypeScript sources

 ## Included Scripts

@@ -32,7 +35,7 @@ The diff should look like this:
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v5.0.0/box-sx-prop.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v5.0.0/box-sx-prop.js ./src

moved-lab-modules

@@ -52,7 +55,7 @@ or


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v5.0.0/moved-lab-modules.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v5.0.0/moved-lab-modules.js ./src

textfield-variant-prop

@@ -70,7 +73,7 @@ The diff should look like this: This codemod is non-idempotent (variant="standard" would be added on a subsequent run, where variant="outlined" was removed), so should only be run once against any particular codebase.

-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v5.0.0/textfield-variant-prop.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v5.0.0/textfield-variant-prop.js ./src

v4.0.0

@@ -86,7 +89,7 @@ The diff should look like this:


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v4.0.0/theme-spacing-api.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v4.0.0/theme-spacing-api.js ./src

This codemod tries to perform a basic expression simplification which can be improved for expressions that use more than one operation. @@ -111,7 +114,7 @@ Converts all @material-ui/core imports more than 1 level deep to the optimal f


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v4.0.0/optimal-imports.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v4.0.0/optimal-imports.js ./src

Head to https://material-ui.com/guides/minimizing-bundle-size/ to understand when it's useful. @@ -127,7 +130,7 @@ Converts all @material-ui/core submodule imports to the root module:


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v4.0.0/top-level-imports.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v4.0.0/top-level-imports.js ./src

Head to https://material-ui.com/guides/minimizing-bundle-size/ to understand when it's useful. @@ -146,7 +149,7 @@ The diff should look like this:


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v1.0.0/import-path.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v1.0.0/import-path.js ./src

Notice: if you are migrating from pre-v1.0, and your imports use material-ui, you will need to manually find and replace all references to m aterial-ui in your code to @material-ui/core. E.g.: @@ -171,7 +174,7 @@ The diff should look like this:


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v1.0.0/color-imports.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v1.0.0/color-imports.js ./src

additional options @@ -193,7 +196,7 @@ The diff should look like this:


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v1.0.0/svg-icon-imports.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v1.0.0/svg-icon-imports.js ./src

v0.15.0

@@ -215,7 +218,7 @@ The diff should look like this:


 ```sh
-find src -name '*.js' -print | xargs npx jscodeshift -t node_modules/@material-ui/codemod/v0.15.0/import-path.js
+npx jscodeshift --extensions tsx,js,ts --parser tsx -t ./node_modules/@material-ui/codemod/node/v0.15.0/import-path.js ./src