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.

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

Bump jscodeshift to ^0.15.2 #34680

Merged
merged 12 commits into from
Apr 11, 2024
10 changes: 10 additions & 0 deletions packages/mui-codemod/codemod.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ function run(argv) {

runJscodeshiftTransform(codemod, files, flags, argv._);
runPostcssTransform(codemod, files);
if (!flags.skipPrettier) {
childProcess.spawnSync('prettier', ['--write', `prettier --write $(git diff --name-only)`], {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a bit heavy to spawn an extra process, wouldn't it be possible as a jscodeshift transformer?

import * as prettier from 'prettier';

export default async function transformer(file) {
  const prettierConfig = await prettier.resolveConfig(file.path);
  return prettier.format(file.source, prettierConfig);
}

Almost feels like those APIs were made for each other 😄

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. We need to add prettier as a dependency, so using npx will be a bit slower, right?
  2. Adding this requires a lot of changes to the expected files. I try to avoid it.

Copy link
Member

@Janpot Janpot Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. there could be a one time cost in some cases, looks like prettier total install size is about 50% of jscodeshift total install size (total size = package + dependencies). I'm not sure npx install time is something we should optimize for here, ideally this will be run at most once ever over the lifetime of a project.
  2. I suppose you can still make it optional with a skipPrettier flag

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. ideally this will be run at most once ever over the lifetime of a project

If the codemod release a new version, isn't another installation is required?

I suppose you can still make it optional with a skipPrettier flag

Yes, it can be skipped.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried timing the installation of jscodeshift alone and also jscodeshift and prettier together and the difference seems to be insignificant (<10ms)

stdio: 'inherit',
});
}
}

yargs
Expand Down Expand Up @@ -194,6 +199,11 @@ yargs
default: false,
type: 'boolean',
})
.option('skip-prettier', {
description: 'skip running prettier on the changed files after the transformation',
default: false,
type: 'boolean',
})
.option('jscodeshift', {
description: '(Advanced) Pass options directly to jscodeshift',
default: false,
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@
"@babel/core": "^7.24.4",
"@babel/runtime": "^7.24.4",
"@babel/traverse": "^7.24.1",
"jscodeshift": "^0.13.1",
"jscodeshift": "^0.15.2",
"jscodeshift-add-imports": "^1.0.10",
"postcss": "^8.4.38",
"postcss-cli": "^8.3.1",
"yargs": "^17.7.2"
},
"devDependencies": {
"@types/chai": "^4.3.14",
"@types/jscodeshift": "0.11.5",
"@types/jscodeshift": "0.11.11",
"chai": "^4.4.1"
},
"sideEffects": false,
Expand Down
Loading