Skip to content

Conversation

@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Aug 14, 2024

closes #43230

<FormControl
  disabled={disabled}
  id={id}
  sx={[...(Array.isArray(formControlSx) ? formControlSx : [formControlSx])]}
/>;

The code above is a type of sx spread which works with MUI System. The codemod should not transform this code.

Unrelated to this PR but need to check if Pigment CSS will skip this code or not.


@mui-bot
Copy link

mui-bot commented Aug 14, 2024

Netlify deploy preview

https://deploy-preview-43291--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 323278f

@siriwatknp siriwatknp requested review from LukasTy and mnajdova August 14, 2024 04:32
@siriwatknp siriwatknp added type: bug It doesn't behave as expected. package: codemod Specific to codemod. v6.x and removed type: bug It doesn't behave as expected. labels Aug 14, 2024
Comment on lines +343 to +346
(data.node.argument.test.type === 'CallExpression' &&
data.node.argument.test.callee.type === 'MemberExpression' &&
data.node.argument.test.callee.object.name === 'Array' &&
data.node.argument.test.callee.property.name === 'isArray') ||
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure that we don't need the sx prop name check in this rule as well? 🤔
I suspect that given the existing rule, it would skip the following prop as well, even though it probably shouldn't. 🤔

xyz={[
  ...(Array.isArray(xyz) ? xs : [xyz]),
  ...(Array.isArray(slotProps?.layout?.xyz) ? slotProps.layout.xyz : [slotProps.layout.sxxyz),

Copy link
Member Author

Choose a reason for hiding this comment

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

This codemod only targets sx prop, so it will not look into this xyz prop.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I saw sx checks in the || else rules and assumed these are the only checks. 🙈
LGTM then, sorry for the distraction. 👍

Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of it. 🙏

Comment on lines +343 to +346
(data.node.argument.test.type === 'CallExpression' &&
data.node.argument.test.callee.type === 'MemberExpression' &&
data.node.argument.test.callee.object.name === 'Array' &&
data.node.argument.test.callee.property.name === 'isArray') ||
Copy link
Member

Choose a reason for hiding this comment

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

Oh, I saw sx checks in the || else rules and assumed these are the only checks. 🙈
LGTM then, sorry for the distraction. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: codemod Specific to codemod. v6.x

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[codemod] v6.0.0/sx-prop produces strange changes

3 participants