search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.9k stars 1.18k forks source link

[data grid] 7.0 migration script exception #12567

Closed gvidaver closed 4 hours ago

gvidaver commented 3 months ago

Steps to reproduce

Link to live example: (required)

Gosh I wish I could give you a runnable example, but here's the log instead. Steps:

  1. npx @mui/x-codemod@next v7.0.0/data-grid/preset-safe .
  2. see exception:
%  npx @mui/x-codemod@next v7.0.0/data-grid/preset-safe .

Need to install the following packages:
@mui/x-codemod@7.0.0-beta.7
Ok to proceed? (y) y
npm WARN deprecated source-map-url@0.4.1: See https://github.com/lydell/source-map-url#deprecated
npm WARN deprecated @babel/plugin-proposal-class-properties@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-class-properties instead.
npm WARN deprecated @babel/plugin-proposal-nullish-coalescing-operator@7.18.6: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-nullish-coalescing-operator instead.
npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated
npm WARN deprecated @babel/plugin-proposal-optional-chaining@7.21.0: This proposal has been merged to the ECMAScript standard and thus this plugin is no longer maintained. Please use @babel/plugin-transform-optional-chaining instead.
npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated
npm WARN deprecated source-map-resolve@0.5.3: See https://github.com/lydell/source-map-resolve#deprecated
Executing command: jscodeshift /Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/node_modules/jscodeshift/bin/jscodeshift.js --transform /Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/v7.0.0/data-grid/preset-safe --extensions js,ts,jsx,tsx --parser tsx --ignore-pattern **/node_modules/** /Users/go22670/Projects/kwicked2

====================================
IMPORTANT NOTICE ABOUT CODEMOD USAGE
====================================
Not all use cases are covered by codemods. In some scenarios, like props spreading, cross-file dependencies and etc., the changes are not properly identified and therefore must be handled manually.

For example, if a codemod tries to rename a prop, but this prop is hidden with the spread operator, it won't be transformed as expected.
<DatePicker {...pickerProps} />

After running the codemods, make sure to test your application and that you don't have any console errors.

Processing 75 files... 
Spawning 9 workers...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 9 files to free worker...
Sending 3 files to free worker...
 ERR /Users/go22670/Projects/kwicked2/src/components/transcript/Transcript.tsx Transformation error (did not recognize object of type "TSInstantiationExpression")
Error: did not recognize object of type "TSInstantiationExpression"
    at Object.getFieldNames (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/types.js:660:19)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:184:36)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at NodePath.each (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path.js:87:26)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:178:18)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at NodePath.each (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path.js:87:26)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:178:18)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at NodePath.each (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path.js:87:26)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:178:18)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at NodePath.each (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path.js:87:26)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:178:18)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at visitChildren (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:203:25)
    at Visitor.PVp.visitWithoutReset (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:166:20)
    at Visitor.PVp.visit (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:104:29)
    at Object.visit (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/ast-types/lib/path-visitor.js:80:55)
    at Collection.<anonymous> (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/node_modules/jscodeshift/src/collections/Node.js:53:14)
    at Array.forEach (<anonymous>)
    at Collection.find (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/node_modules/jscodeshift/src/collections/Node.js:44:18)
    at Collection.find (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/node_modules/jscodeshift/src/Collection.js:413:43)
    at checkPreRequisitesSatisfied (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/util/renameIdentifiers.js:36:26)
    at transformer (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/v6.0.0/data-grid/rename-components-to-slots/index.js:40:72)
    at transformer (/Users/go22670/.npm/_npx/61f2f8787e4fed1c/node_modules/@mui/x-codemod/v7.0.0/data-grid/preset-safe/index.js:11:53)

All done. 
Results: 
1 errors
72 unmodified
0 skipped
2 ok
Time elapsed: 39.960seconds

Current behavior

Throws an exception while running migration script. Seems to complete otherwise though.

Expected behavior

No exception.

Context

Running 7.0 migration script.

Your environment

npx @mui/envinfo ``` System: OS: macOS 14.4 Binaries: Node: 21.7.1 - /usr/local/bin/node npm: 10.5.0 - /usr/local/bin/npm pnpm: Not Found Browsers: Chrome: 123.0.6312.59 Edge: Not Found Safari: 17.4 npmPackages: @emotion/react: ^11.11.3 => 11.11.3 @emotion/styled: ^11.11.0 => 11.11.0 @mui/base: 5.0.0-beta.35 @mui/core-downloads-tracker: 5.15.8 @mui/icons-material: ^5.15.7 => 5.15.7 @mui/joy: ^5.0.0-beta.26 => 5.0.0-beta.26 @mui/material: ^5.14.7 => 5.14.7 @mui/private-theming: 5.15.8 @mui/styled-engine: 5.15.8 @mui/system: 5.15.8 @mui/types: 7.2.13 @mui/utils: 5.15.8 @mui/x-data-grid: next => 6.19.2 @mui/x-data-grid-generator: next => 6.19.2 @mui/x-data-grid-premium: 6.19.2 @mui/x-data-grid-pro: 6.19.2 @mui/x-date-pickers: 6.19.2 @mui/x-date-pickers-pro: next => 6.19.2 @mui/x-license-pro: 6.10.2 @types/react: ^18.2.48 => 18.2.48 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ^5.3.3 => 5.3.3 ```

Search keywords: TSInstantiationExpression Order ID: c74f7af5c579dc47e7ed444056faa516Tz03NzAwMCxFPTE3MjkzNjI5NDQwMDAsUz1wcm8sTE09c3Vic2NyaXB0aW9uLEtWPTI=

michelengelen commented 3 months ago

Hey @gvidaver could you tell us what the content of /Users/go22670/Projects/kwicked2/src/components/transcript/Transcript.tsx is?

Janpot commented 3 months ago

Looks like the codemod struggles with modern typescript constructs, like satisfies or instantiation expressions. Seeing it on the v7 migration in Toolpad as well

MBilalShafi commented 3 months ago

The reason migration on some of the recently introduced typescript constructs fails is the usage of the previous version of the jscodeshift which doesn't support them, the bump to the latest version is blocked due to an issue with the underlying library.

We will try to have a workaround by pinning the dependencies to a previous version using yarn resolutions as suggested in https://github.com/facebook/jscodeshift/issues/534#issuecomment-1344735177

roastnewt commented 2 months ago

+1

Janpot commented 2 months ago

the bump to the latest version is blocked due to an issue with the underlying library.

I see. Personally, that issue feels less of a problem to me than the typescript one. It can be easily solved on my end by running my formatter after the MUI codemod.

It's also not the only parentheses formatting problem with codemod. I ran into the following as well on the Toolpad codebase:

const x = () => ({ foo: 'bar' }) as Foo;

gets turned into

const x = () => ({ foo: 'bar' } as Foo);
github-actions[bot] commented 2 months ago

The issue has been inactive for 7 days and has been automatically closed.

Janpot commented 2 months ago

Reproductions for both cases:

import { GridColDef } from "@mui/x-data-grid";

// Transformation error (did not recognize object of type "TSSatisfiesExpression")
const cols = [] satisfies GridColDef[];
function makeBox<T>(value: T) {
  return { value };
}

// Transformation error (did not recognize object of type "TSInstantiationExpression")
const makeStringBox = makeBox<string>; 
const stringBox = makeStringBox("abc");
Nick-Lucas commented 2 months ago

I see. Personally, that issue feels less of a problem to me than the typescript one. It can be easily solved on my end by running my formatter after the MUI codemod.

Agreed, perhaps the team could publish a canary with the upgrade on, that way those of us who use formatters can try the codemod and at least feed back on any other issues which come out the woodwork? Best case it works great alongside prettier and we can get upgraded!

Nick-Lucas commented 2 weeks ago

Morning @MBilalShafi @Janpot was there any progress on this?

My team (Premium licences) is keen to upgrade, and can easily run prettier formatting afterwards, but the codemod crashing is a blocker as we have too many datagrids to migrate manually

github-actions[bot] commented 4 hours ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@gvidaver: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.