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

Bump jscodeshift to ^0.15.2 #34680

Merged
merged 12 commits into from
Apr 11, 2024
Merged

Bump jscodeshift to ^0.15.2 #34680

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
865372d
Bump jscodeshift to ^0.15.2
renovate[bot] Apr 3, 2024
a64976c
fix all expected tests
siriwatknp Apr 10, 2024
e91f8ed
Merge branch 'next' of https://github.com/mui/material-ui into renova…
siriwatknp Apr 11, 2024
cfa055e
run prettier after transformation
siriwatknp Apr 11, 2024
43370da
Merge branch 'renovate/jscodeshift-0.x' of https://github.com/mui/mat…
siriwatknp Apr 11, 2024
7f7d2c2
Merge branch 'next' of https://github.com/mui/material-ui into renova…
siriwatknp Apr 11, 2024
7050c34
run prettier after jscodeshift
siriwatknp Apr 11, 2024
6c66615
fix lint
siriwatknp Apr 11, 2024
802c94e
use execSync
siriwatknp Apr 11, 2024
e51daeb
add try catch
siriwatknp Apr 11, 2024
5e2d461
fix lint
siriwatknp Apr 11, 2024
92b8a2a
revert prettier
siriwatknp Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions packages/mui-codemod/codemod.js
View file
Open in desktop
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 •
edited
Loading

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