[codemod] Add sx prop for v6 migration

[siriwatknp]

Review suggestion

Please see the test cases (*.actual and *.expected). Feel free to suggest more test cases.

Implementation

Reuses some functions from styled and theme codemods. The logic is similar but require special treatment for some scenarios, e.g. there is no variants in sx prop.

• I have followed (at least) the PR section of the contributing guide.

[mui-bot]

Netlify deploy preview

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

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 7dc352f

[mnajdova]

Some initial feedback and thoughts.

[mnajdova]

Shouldn't this come last?

[siriwatknp]

No, the generated CSS vars should come last to ensure that it takes effect. It'd be 99.9999% that the CSS vars from props.style will get overridden.

[mnajdova]

I don't understand the argument honestly. Regardless of what the sx props generated, the users's style attribute should win, even if we almost never have cases where people need to do this.

[siriwatknp]

I'm fine to change. As a note, it does not matter which way we choose to override (before or after) because if the generated variable is the same as their existing style.*, the result styles will not be the same but as I said, that's 99.9999%.

For example,
Before:

<Box
  style={{
    '--items-imageLight': '10px', // existing user's defined variable 
  }}
  sx={{
    fontSize: 'var(--items-imageLight)',
    backgroundImage: (theme) =>
      theme.palette.mode === 'light'
        ? items[selectedItemIndex].imageLight
        : items[selectedItemIndex].imageDark,
  }}
/>;

After:

<Box
  style={{
    "--items-imageLight": items[selectedItemIndex].imageLight,
    "--items-imageDark": items[selectedItemIndex].imageDark,
    '--items-imageLight': '10px', // existing user's defined variable
  }}
  sx={theme => ({
    fontSize: 'var(--items-imageLight)',
    backgroundImage: "var(--items-imageDark)",
    ...theme.applyStyles("light", {
      backgroundImage: "var(--items-imageLight)", // ❌
    })
  })}
/>;

[DiegoAndai]

Sorry for the delay on the review @siriwatknp

[DiegoAndai]

Are these empty object required?

[siriwatknp]

Let me improve on this.

[siriwatknp]

Done! Please check 0552e8b#diff-b8f0d8f47671f8e415e56742ab13b840f03f7c539d6fc2e57b20fd2e4d7d422a

[siriwatknp]

@mnajdova @DiegoAndai Could you do another review?

[DiegoAndai]

Looks good! Only one small nit pick

[DiegoAndai]

Could use: https://github.com/mui/material-ui/blob/next/packages/mui-codemod/testUtils/index.js#L13

[siriwatknp]

I did not know it existed. To be honest, I prefer writing test separately instead of using .forEach. The reason is debugging.

With multiple it(), you can use .skip or .only to debug a test (I frequently do this all the time), but it's not possible with .forEach. If you have a way to debug each test but still using .forEach, I'm all ears.