Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[codemod] Not able to use other parser #29059

Closed
2 tasks done
jennykortina opened this issue Oct 14, 2021 · 11 comments
Closed
2 tasks done

[codemod] Not able to use other parser #29059

jennykortina opened this issue Oct 14, 2021 · 11 comments
Labels
new feature New feature or request package: codemod Specific to @mui/codemod

Comments

@jennykortina
Copy link

jennykortina commented Oct 14, 2021 •
edited
Loading

I am following the https://mui.com/guides/migration-v4/ guide. When I run npx @mui/codemod v5.0.0/preset-safe ./ --no-babel --parser=flow I get the following error:

 Transformation error (Unexpected token (40:11))
SyntaxError: Unexpected token (40:11)
    at Object._raise (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/error.js:147:45)
    at Object.raiseWithData (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/error.js:142:17)
    at Object.raise (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/error.js:91:17)
    at Object.unexpected (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/util.js:190:16)
    at Object.tsParseNonArrayType (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1071:18)
    at Object.tsParseArrayTypeOrHigher (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1075:23)
    at Object.parseConstituentType [as tsParseTypeOperatorOrHigher] (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1133:16)
    at Object.tsParseUnionOrIntersectionType (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1145:20)
    at Object.parseConstituentType [as tsParseIntersectionTypeOrHigher] (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1155:19)
    at Object.tsParseUnionOrIntersectionType (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1145:20)
  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When I do a stack trace to the line it's failing on it's a "maybe" flow type eg days: ?number and if I fix that type to not be a maybe, it will crash on the next "maybe" flow type.

Expected Behavior 🤔

I would expect it to run on maybe flow types without error

Steps to Reproduce 🕹

Steps:

  1. run npx @mui/codemod v5.0.0/preset-safe ./ --no-babel --parser=flowon a component with a maybe flow type egdays: ?number`

Context 🔦

I am trying to upgrade material UI from 4 -> 5
@jennykortina jennykortina added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Oct 14, 2021
@siriwatknp siriwatknp added new feature New feature or request package: codemod Specific to @mui/codemod and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Oct 15, 2021
@siriwatknp
Copy link
Member

Can you provide one of the sample file with flow type?

@jennykortina
Copy link
Author

Sure if I have a type like this:

type P = {
  months: number,
  days: number,
  hours: ?number,
  minutes: number,
};

it would break on the hours: ?number. It breaks anywhere in the codebase that a maybe flow type is used. Here is a link to maybe flow types docs. Again the error from above (also pasted below) is what we see when trying to run the script Transformation error (Unexpected token...:

Transformation error (Unexpected token (40:11))
SyntaxError: Unexpected token (40:11)
    at Object._raise (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/error.js:147:45)
    at Object.raiseWithData (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/error.js:142:17)
    at Object.raise (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/error.js:91:17)
    at Object.unexpected (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/parser/util.js:190:16)
    at Object.tsParseNonArrayType (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1071:18)
    at Object.tsParseArrayTypeOrHigher (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1075:23)
    at Object.parseConstituentType [as tsParseTypeOperatorOrHigher] (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1133:16)
    at Object.tsParseUnionOrIntersectionType (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1145:20)
    at Object.parseConstituentType [as tsParseIntersectionTypeOrHigher] (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1155:19)
    at Object.tsParseUnionOrIntersectionType (/Users/jennykortina/.npm/_npx/24417/lib/node_modules/@mui/codemod/node_modules/@babel/parser/src/plugins/typescript/index.js:1145:20)

Thank you so much!

@ElonVolo
Copy link
Contributor

Putting in a PR for this

ElonVolo added a commit to ElonVolo/material-ui that referenced this issue Oct 22, 2021
@ElonVolo
[codemod] Add parser flag (mui#29059)
b5e35a7
@ElonVolo ElonVolo mentioned this issue Oct 22, 2021
Merged
@ElonVolo
Copy link
Contributor

ElonVolo commented Oct 22, 2021 •
edited
Loading

There was an issue with how the codemod was being run. 'tsx' was hardcoded in the const args = [ array in packages/mui-codemod/codemod.js

You can temporarily fix the problem by changing

    '--parser',  
    'tsx',

to

   '--parser',  
   'flow',

I put in a PR to fix this. #29229

If someone could see why circleci is breaking on it that would be great. Maybe something has to be overridden for markdown docs.

--
This bug was troubleshot with the logitall dumbtracer. https://github.com/ElonVolo/logitall

@siriwatknp
Copy link
Member

If someone could see why circleci is breaking on it that would be great. Maybe something has to be overridden for markdown docs.

I can help with that 👍

@jennykortina
Copy link
Author

Thank you!

@ElonVolo
Copy link
Contributor

@jennykortina, as a real-world user who uses flowtype with mui and who probably had way too much of their time devoured by this bug, would you please weigh in on #29229 and give us your perspective on what you expect from the mui codehelper and how the interface to it should be handled.

@jennykortina
Copy link
Author

@ElonVolo cool cool thank you for the help! I imagine a lot of flow and material users are having this same issue as a maybe flow type is very common (it's just saying the type maybe null).

For the temporary fix are you saying to run it like this?

npx @mui/codemod v5.0.0/preset-safe ./ --no-babel --parser=flow

B/c i've tried that and it does not work. Or do I need to pass the parser flag differently?

Thank you!

@ElonVolo
Copy link
Contributor

@jennykortina, there were two basic problems you ran into:

  1. The codemod script doesn't take a --parser argument.
  2. Even if it did, it probably wouldn't matter, because the script hardcodes tsx as the parser that is passed to the internal jscodeshift calls.

For your temp workaround to work you literally have to go into the code of codemod.js with your editor of choice and manually change "tsx" to "flow". Maybe other's can shed a light on whether there's some existing override that would work (maybe pass something in the jscodeshift parameter). In any case, if there is a workaround that would work for flow, it should probably be explicit documented.

But it seems like there's no super duper, easy, jumps-out-and-grabs-you way for someone to migrate a flowtype based project. e.g.

npx @mui/codemod v5.0.0/preset-safe --typesystem flow

@jennykortina
Copy link
Author

@ElonVolo ok thank you I will give that a try and report back so other's know if this works. thanks!

mnajdova pushed a commit that referenced this issue Oct 29, 2021
@ElonVolo
[codemod] Add codemod parser flag (#29059) (#29229)
6926938
@siriwatknp
Copy link
Member

This is fixed in https://github.com/mui-org/material-ui/releases/tag/v5.1.0

@siriwatknp siriwatknp changed the title Transformation error Unexpected token when running preset-safe codemod [codemod] Support custom parser Nov 26, 2021
@siriwatknp siriwatknp changed the title [codemod] Support custom parser [codemod] Able to pass other parser Nov 26, 2021
@siriwatknp siriwatknp changed the title [codemod] Able to pass other parser [codemod] Not able to use other parser Nov 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
new feature New feature or request package: codemod Specific to @mui/codemod
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jennykortina @siriwatknp @ElonVolo