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.

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

[Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components #34997

Merged
merged 147 commits into from Dec 5, 2022

Conversation

hbjORbj
Copy link
Member

@hbjORbj hbjORbj commented Nov 3, 2022

This PR includes:

  • Rename components to slots and componentsProps to slotProps in all Joy UI components and in useSlot().
  • Apply Joy useSlot() to all Joy UI components
  • Apply slot test to all Joy UI components
  • Add spec tests to all Joy UI components
  • Codemod for renaming components to slots and componentsProps to slotProps in all Joy UI components

Checklist

  • Alert
  • AspectRatio
  • Avatar
  • AvatarGroup
  • Badge
  • Box
  • Breadcrumbs
  • Button
  • Card
  • CardContent
  • CardCover
  • CardOverflow
  • Checkbox
  • Chip
  • ChipDelete
  • CircularProgress
  • Container
  • CssBaseline
  • Divider
  • FormControl
  • FormHelperText
  • FormLabel
  • Grid
  • IconButton
  • Input
  • LinearProgress
  • Link
  • List
  • ListDivider
  • ListItem
  • ListItemButton
  • ListItemContent
  • ListItemDecorator
  • ListSubheader
  • Menu
  • MenuItem
  • MenuList
  • Modal
  • ModalClose
  • ModalDialog
  • Option
  • Radio
  • RadioGroup
  • ScopedCssBaseline
  • Select
  • Sheet
  • Slider
  • Stack
  • SvgIcon
  • Switch
  • Tab
  • TabList
  • TabPanel
  • Tabs
  • Textarea
  • TextField
  • Tooltip
  • Typography

@hbjORbj hbjORbj self-assigned this Nov 3, 2022
@hbjORbj hbjORbj added breaking change package: joy-ui Specific to @mui/joy labels Nov 3, 2022
@mui-bot
Copy link

mui-bot commented Nov 3, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-34997--material-ui.netlify.app/

@mui/joy: parsed: +0.15% , gzip: +0.14%

Details of bundle changes

Generated by 🚫 dangerJS against 715d72d

@hbjORbj hbjORbj changed the title Draft: [Joy] Apply useSlot to components and add slots/slotProps props to the typing Draft: [Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components Nov 3, 2022
@hbjORbj hbjORbj changed the title Draft: [Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components Draft: [Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components Nov 3, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 8, 2022
@hbjORbj hbjORbj changed the title Draft: [Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components [Joy] Add slots/slotProps props to the typing of all components and apply useSlot to all components Nov 8, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 8, 2022
@hbjORbj hbjORbj force-pushed the joy/apply-useSlot branch 4 times, most recently from f15301f to d2e812a Compare November 17, 2022 10:31
@hbjORbj
Copy link
Member Author

hbjORbj commented Nov 17, 2022

A few tests still fail, but I think I spent enough time trying to figure out. Your help would be appreciated, Jun. @siriwatknp

@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged and removed PR: out-of-date The pull request has merge conflicts and can't be merged labels Nov 17, 2022
Copy link
Member

@siriwatknp siriwatknp left a comment

Choose a reason for hiding this comment

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

👍 Great job @hbjORbj. Let's wait until the codemod PR is approved.

const root = j(file.source);
const printOptions = options.printOptions;

const transformed = root.findJSXElements().forEach((path) => {
Copy link
Member

Choose a reason for hiding this comment

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

@hbjORbj To make codemod specific to Joy UI. We should do this:

  1. list the component imports first
import { Alert as JoyAlert } from '@mui/joy';
import JoyAutocomplete from '@mui/joy/Autocomplete';

// the list should be ['JoyAlert', 'JoyAutocomplete']
  1. use .findJSXElements($name) and then do the transform as you already did.

Copy link
Member

@siriwatknp siriwatknp Dec 2, 2022

Choose a reason for hiding this comment

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

Copy link
Member Author

@hbjORbj hbjORbj Dec 4, 2022

Choose a reason for hiding this comment

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

Hey, Thanks, Jun.

I changed it to the following in this commit. Please let me know if I did it correctly.

  root
    .find(j.ImportDeclaration)
    .filter(({ node }) => {
      return node.source.value.startsWith('@mui/joy');
    })
    .forEach((path) => {
      path.node.specifiers.forEach((node) => {
        root.findJSXElements(node.local.name).forEach((elementPath) => {
          if (elementPath.node.type !== 'JSXElement') {
            return;
          }

          elementPath.node.openingElement.attributes.forEach((elementNode) => {
            if (elementNode.type !== 'JSXAttribute') {
              return;
            }

            switch (elementNode.name.name) {
              case 'components':
                transformComponentsProp(elementNode);
                break;

              case 'componentsProps':
                transformComponentsPropsProp(elementNode);
                break;

              default:
            }
          });
        });
      });
    });

  const transformed = root.findJSXElements();
  return transformed.toSource(printOptions);

@siriwatknp
Copy link
Member

@hbjORbj I suggest adding another test case for the codemod.

// the codemod should transform only Joy UI components;
import { Alert as JoyAlert } from '@mui/joy';
import JoyAutocomplete from '@mui/joy/Autocomplete';

import CustomComponent from 'components/Custom';

function App() {
  return (
    <div>
      <JoyAlert components={{ ... }} componentsProps={{ ... }} />
      <JoyAutocomplete components={{ ... }} componentsProps={{ ... }} />
      <CustomComponent components={{ ... }} componentsProps={{ ... }} />
    </div>
  )
}

@hbjORbj
Copy link
Member Author

hbjORbj commented Dec 4, 2022

Ready for review again! @siriwatknp

@hbjORbj hbjORbj merged commit 2bf4c80 into mui:master Dec 5, 2022
@hbjORbj hbjORbj removed the on hold There is a blocker, we need to wait label Dec 5, 2022
feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants