-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[mui-codemod] Fix findComponentJSX
util
#40855
Conversation
Netlify deploy previewhttps://deploy-preview-40855--material-ui.netlify.app/ Bundle size report |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this @sai6855
.filter( | ||
(path) => | ||
path.node.source.value.match(new RegExp(`^@mui/material/(${componentName})`)) || | ||
path.node.source.value === '@mui/material', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can make it simpler:
.filter( | |
(path) => | |
path.node.source.value.match(new RegExp(`^@mui/material/(${componentName})`)) || | |
path.node.source.value === '@mui/material', | |
.filter((path) => | |
path.node.source.value.match(new RegExp(`^@mui/material(/${componentName})?$`)), |
It's the same as your fix, as $
indicates to match the end of the line, so it's either
- string start (
^
) ->@mui/material
-> string end ($
) - string start (
^
) ->@mui/material/componentName
-> string end ($
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clear explaination. fixed in this commit 8b714f8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Did you test if it's fixed on your side after the refactor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, ran codemod script. Now util is filtering out data correctly
How to test the fix
console.log(elementPath.node.openingElement.name.name);
below this linematerial-ui/packages/mui-codemod/src/deprecations/accordion-props/accordion-props.js
Line 14 in 97f225c
node packages/mui-codemod/codemod deprecations/accordion-props packages/mui-codemod/src/deprecations/accordion-props/test-cases/actual.js
Accordion
2 times, everything is good until nowcomponentName: "Accordion"
tocomponentName: "sank" (or any random value)
on this line and runnode packages/mui-codemod/codemod deprecations/accordion-props packages/mui-codemod/src/deprecations/accordion-props/test-cases/actual.js
Accordion
again 2 times, as per my understandingfindComponentJSX
function should filter out components which matches withcomponentName
but i don't think that's the case now. This PR fixes itNote: I'm really new to
codemods
andRegex
, apologies in advance if my understanding or logic is wrong