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

Bump jscodeshift to ^0.15.2 #34680

Merged
merged 12 commits into from
Apr 11, 2024
Merged

Bump jscodeshift to ^0.15.2 #34680

Show file tree
Hide file tree
Changes from 10 commits
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
13 changes: 13 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 @@ -83,6 +83,14 @@ async function runJscodeshiftTransform(transform, files, flags, codemodFlags) {

if (jscodeshiftProcess.error) {
throw jscodeshiftProcess.error;
} else if (!flags.skipPrettier) {
try {
childProcess.execSync('prettier --write $(git diff --name-only)', {
stdio: 'inherit',
});
} catch (error) {
// silentyly ignore errors
}
Copy link
Member

@siriwatknp siriwatknp 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.

Tested, with the CSB build, it works fine. @Janpot what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Personally I don't think we should do it this way. I suppose the idea of ignoring all errors is to ignore when prettier is not installed. But this will also silence any other useful error. For instance, in large projects this will potentially generate a large amount of file names, which can result in failures in the shell as there are limits to the command length. We've ran into this ourselves with code infra with our prettier command and this is the reason why we now use a tool that runs prettier programmatically.

I think we should run prettier programmatically on every file if:

  1. there is a straightforward way to run it e.g. through a jscodeshift transformer
  2. there is a straightforward way to turn it off with a flag
  3. there is a straightforward way to turn it off when no prettier config exists in the project

If any of these 3 conditions is impossible, I'd consider not adding prettier formatting at all.

Copy link
Member

Choose a reason for hiding this comment

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

which can result in failures in the shell as there are limits to the command length

I think it's an edge case because the prettier only runs on changed files. I used to run codemod on several large projects (backstage is one of them) but I've never found a case where file changes go beyond hundreds before.

Anyway, in case there is an error, --skip-prettier can be used.

Copy link
Member

Choose a reason for hiding this comment

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

What we could do is to not add prettier at first and see the feedback. I will remove it for now.

Copy link
Member

Choose a reason for hiding this comment

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

I ended up here after running the deprecations codemod in our docs. I had some files present the double parens issue. In my case, it was easy to bypass running prettier manually. It wasn't that bad. I wouldn't complain if prettier was ran automatically, maybe as an opt-in.

Copy link
Member

Choose a reason for hiding this comment

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

We should at least add a warning for this in the codemod readme.

}
}

Expand Down Expand Up @@ -194,6 +202,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
View file
Open in desktop
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
9 changes: 4 additions & 5 deletions packages/mui-codemod/src/v5.0.0/base-remove-unstyled-suffix.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ export default function transformer(file, api) {
.find(j.ImportDeclaration)
.filter(({ node }) => {
const sourceVal = node.source.value;
if (sourceVal.startsWith('@mui/base')) {
node.source.value = sourceVal.replace(/unstyled/im, '');
node.source.raw = sourceVal.replace(/unstyled/im, '');
}

return sourceVal.startsWith('@mui/base');
})
.forEach((path) => {
const sourceVal = path.node.source.value;
if (sourceVal.startsWith('@mui/base')) {
path.node.source = j.stringLiteral(sourceVal.replace(/unstyled/im, ''));
}
const specifiers = [];
path.node.specifiers.forEach((elementNode) => {
const importedName = elementNode.imported?.name || '';
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/box-sx-prop.test/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Button from '@material-ui/core/Button';

export default function BoxComponent(props) {
return (
<Box
(<Box
sx={{
m: 2,
border: "1px dashed grey",
Expand All @@ -25,6 +25,6 @@ export default function BoxComponent(props) {
sx={{
p: [1, 2, 4]
}} />
</Box>
</Box>)
);
}
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/eighth.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export default function Cart() {
});
};
return (
<Root>
(<Root>
<Head title="View cart">
<meta name="robots" content="noindex,nofollow" />
</Head>
Expand Down Expand Up @@ -208,6 +208,6 @@ export default function Cart() {
</OrderBody>
</Container>
<AppFooter />
</Root>
</Root>)
);
}
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/eleventh.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ export default function Page() {


return (
<StyledSomeNamespaceSomeComponent>
(<StyledSomeNamespaceSomeComponent>
<h1 className={classes.header}></h1>
<img className={classes.img}></img>
</StyledSomeNamespaceSomeComponent>
</StyledSomeNamespaceSomeComponent>)
);
}
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/fifth.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function SellHero() {


return (
<Root className={classes.root}>
(<Root className={classes.root}>
<Container className={classes.container}>
<Typography variant="h1" align="center" color="textPrimary" className={classes.title}>
Sell themes
Expand All @@ -87,7 +87,7 @@ function SellHero() {
</Button>
</div>
</Container>
</Root>
</Root>)
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/first.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class AppAppBar extends React.Component {
const { menuOpen } = this.state;

return (
<StyledAppBar essential={essential} position={position}>
(<StyledAppBar essential={essential} position={position}>
<div className={clsx(classes.wrap, { [classes.wrapOpened]: menuOpen })}>
<Link to="/" aria-label="Back to homepage" color="inherit">
<Logo color={menuOpen ? 'inherit' : 'textPrimary'} />
Expand Down Expand Up @@ -237,7 +237,7 @@ class AppAppBar extends React.Component {
</div>
)}
</div>
</StyledAppBar>
</StyledAppBar>)
);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/fourth.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ const StyledCard = styled(Card)(function getStyles(
export const MyCard = ((props) => {
const { } = props;
return (
<StyledCard className={classes.root}>
(<StyledCard className={classes.root}>
<CardActions className={classes.actions}>
<Button className={classes.button}>Submit</Button>
</CardActions>
</StyledCard>
</StyledCard>)
);
});
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/nineth.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ export default function Page() {


return (
<Root>
(<Root>
<h1 className={classes.header}></h1>
<img className={classes.img}></img>
</Root>
</Root>)
);
}
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/second.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function AffiliatesHero() {


return (
<Root className={classes.root}>
(<Root className={classes.root}>
<Container className={classes.container}>
<Typography variant="h1" align="center" color="textPrimary" className={classes.title}>
Affiliate Program
Expand All @@ -85,7 +85,7 @@ function AffiliatesHero() {
</Button>
</div>
</Container>
</Root>
</Root>)
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/seventh.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function Iframe(props) {
}, []);

return (
<Root>
(<Root>
{loaded === false ? (
<div className={classes.loader}>
<CircularProgress />
Expand All @@ -109,7 +109,7 @@ function Iframe(props) {
frameBorder="0"
{...props}
/>
</Root>
</Root>)
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-styled.test/third.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const StyledCard = styled(Card)((
export const MyCard = ((props) => {
const { } = props;
return (
<StyledCard className={classes.root}>
(<StyledCard className={classes.root}>
<CardActions className={classes.actions}>
<Button className={classes.button}>Submit</Button>
</CardActions>
</StyledCard>
</StyledCard>)
);
});
2 changes: 1 addition & 1 deletion packages/mui-codemod/src/v5.0.0/jss-to-styled.test/withCreateStyles.expected.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const Root = styled('div')((
const MyComponent = (props) => {


return <Root {...props} className={classes.root} />;
return (<Root {...props} className={classes.root} />);
};

export default MyComponent;
2 changes: 1 addition & 1 deletion packages/mui-codemod/src/v5.0.0/jss-to-styled.test/withCreateStyles1.expected.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const Root = styled('div')((
const MyComponent = (props) => {
const { } = props;

return <Root {...props} className={classes.root} />;
return (<Root {...props} className={classes.root} />);
};

export default (MyComponent);
2 changes: 1 addition & 1 deletion packages/mui-codemod/src/v5.0.0/jss-to-styled.test/withCreateStyles2.expected.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const Root = styled('div')({
const MyComponent = (props) => {
const { } = props;

return <Root {...props} className={classes.root} />;
return (<Root {...props} className={classes.root} />);
};

export default (MyComponent);
2 changes: 1 addition & 1 deletion packages/mui-codemod/src/v5.0.0/jss-to-styled.test/withCreateStyles3.expected.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const Root = styled('div')({
const MyComponent = (props) => {


return <Root {...props} className={classes.root} />;
return (<Root {...props} className={classes.root} />);
};

export default MyComponent;
9 changes: 5 additions & 4 deletions .../mui-codemod/src/v5.0.0/jss-to-tss-react.test/expected-typescript-docs-example-params.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { makeStyles } from 'tss-react/mui';

const useStyles = makeStyles<{color: 'primary' | 'secondary', padding: number}, 'child' | 'small'>({name: 'App'})((theme, { color, padding }, classes) => ({
root: {
root: ({
padding: padding,

[`&:hover .${classes.child}`]: {
backgroundColor: theme.palette[color].main,
}
},
}),
small: {},
child: {
border: '1px solid black',
Expand All @@ -28,15 +29,15 @@ function App({classes: classesProp}: {classes?: any}) {
});

return (
<div className={classes.root}>
(<div className={classes.root}>
<div className={classes.child}>
The Background take the primary theme color when the mouse hovers the parent.
</div>
<div className={cx(classes.child, classes.small)}>
The Background take the primary theme color when the mouse hovers the parent.
I am smaller than the other child.
</div>
</div>
</div>)
);
}

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/jss-to-tss-react.test/expected-typescript-docs-example.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ const useStyles = makeStyles<void, 'child' | 'small'>()((theme, _params, classes
function App() {
const { classes, cx } = useStyles();
return (
<div className={classes.parent}>
(<div className={classes.parent}>
<div className={classes.child}>
Background turns red when the mouse hovers over the parent.
</div>
<div className={cx(classes.child, classes.small)}>
Background turns red when the mouse hovers over the parent.
I am smaller than the other child.
</div>
</div>
</div>)
);
}

Expand Down
12 changes: 6 additions & 6 deletions packages/mui-codemod/src/v5.0.0/preset-safe.test/expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ const Header = () => {
const classes = useStyles();
const { dark, setDark } = React.useContext(DarkContext);
return (
<AppBar color="default" position="sticky">
(<AppBar color="default" position="sticky">
<Toolbar>
<Typography className={classes.head} variant="h6">
👋 Hello
Expand All @@ -110,7 +110,7 @@ const Header = () => {
onChange={(event, checked) => setDark(checked)}
/>
</Toolbar>
</AppBar>
</AppBar>)
);
};

Expand All @@ -119,7 +119,7 @@ function App() {
const handleClose = () => setOpen(false);
const { setDark } = React.useContext(DarkContext);
const classes = useStyles();
return <>
return (<>
<CssBaseline />
<Header />
<Container>
Expand Down Expand Up @@ -173,7 +173,7 @@ function App() {
</DialogActions>
</Dialog>
</Container>
</>;
</>);
}

const withThemeProvider = (Component) => (props) => {
Expand All @@ -188,15 +188,15 @@ const withThemeProvider = (Component) => (props) => {
[dark],
);
return (
<DarkContext.Provider value={{ dark, setDark }}>
(<DarkContext.Provider value={{ dark, setDark }}>
<StylesProvider injectFirst>
<StyledEngineProvider injectFirst>
<ThemeProvider theme={theme}>
<Component {...props} />
</ThemeProvider>
</StyledEngineProvider>
</StylesProvider>
</DarkContext.Provider>
</DarkContext.Provider>)
);
};

Expand Down
4 changes: 2 additions & 2 deletions packages/mui-codemod/src/v5.0.0/styled-engine-provider.test/mui-theme-provider.expected.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ import Page from './pages';

const App = () => {
return (
<Providers>
(<Providers>
<StyledEngineProvider injectFirst>
<MuiThemeProvider theme={createMuiTheme()}>
<OtherProvider>
<Page />
</OtherProvider>
</MuiThemeProvider>
</StyledEngineProvider>
</Providers>
</Providers>)
);
};

Expand Down
Loading
Loading