makeStyles vs withStyles (with Example) without react refs

[jeremy-coleman]

going forward is makeStyles what you're looking to do?

seems a lot easier than all the ref spam? is there any reason NOT to do this ? asking as both from a material-ui perspective and authoring components externally

I randomly picked the Avatar as an example and just put it inside one of the example dashboard apps..
attached a pic of devtools passing mui name correctly

import React from 'react';
import clsx from 'clsx';
import {withStyles, makeStyles} from '@material-ui/styles';

export const useAvatarCss = makeStyles(theme => ({
  /* Styles applied to the root element. */
  root: {
    position: 'relative',
    display: 'flex',
    alignItems: 'center',
    justifyContent: 'center',
    flexShrink: 0,
    width: 40,
    height: 40,
    fontFamily: theme.typography.fontFamily,
    fontSize: theme.typography.pxToRem(20),
    borderRadius: '50%',
    overflow: 'hidden',
    userSelect: 'none',
  },
  /* Styles applied to the root element if there are children and not `src` or `srcSet`. */
  colorDefault: {
    color: theme.palette.background.default,
    backgroundColor:
      theme.palette.type === 'light' ? theme.palette.grey[400] : theme.palette.grey[600],
  },
  /* Styles applied to the img element if either `src` or `srcSet` is defined. */
  img: {
    width: '100%',
    height: '100%',
    textAlign: 'center',
    // Handle non-square image. The property isn't supported by IE 11.
    objectFit: 'cover',
  },
  system: {}
}), { name: 'MuiAvatar' })


type AvatarBaseProps = {
  alt?: string;
  childrenClassName?: string;
  imgProps?: React.HtmlHTMLAttributes<HTMLImageElement>;
  sizes?: string;
  src?: string;
  srcSet?: string;
  component?: any
} & React.HTMLProps<any>

export function Avatar(props: AvatarBaseProps) {
  const {
    alt,
    children: childrenProp,
    childrenClassName: childrenClassNameProp,
    //classes,
    className: classNameProp,
    component: Component,
    imgProps,
    sizes,
    src,
    srcSet,
    ...other
  } = props;

  const classes = useAvatarCss()
  let children = null;
  const img = src || srcSet;

  if (img) {
    children = (
      <img
        alt={alt}
        src={src}
        srcSet={srcSet}
        sizes={sizes}
        className={classes.img}
        {...imgProps}
      />
    );
  } else if (childrenClassNameProp && React.isValidElement<any>(childrenProp)) {
    children = React.cloneElement<any, any>((childrenProp as any), {
      className: clsx(childrenClassNameProp, childrenProp.props.className),
    });
  } else {
    children = childrenProp;
  }

  return (
    <Component
      className={clsx(classes.root, classes.system, {
        [classes.colorDefault]: !img,
      }, classNameProp)}
      //ref={ref}
      {...other}
    >
      {children}
    </Component>
  );
}

 Avatar.defaultProps = {
  name: 'MuiAvatar',
  component: 'div',
};

export default Avatar

[jeremy-coleman]

[oliviertassinari]

@jeremy-coleman makeStyles and withStyles have their own use cases. I would use the API that reduces boilerplate. In doubt, I would use makeStyles over withStyles. The main use case of withStyles is for building custom variations of our components. To answer your question, yes, it's fine 👍.

[eps1lon]

seems a lot easier than all the ref spam?

Could you elaborate what you mean by that?

[jeremy-coleman]

thanks olivier, @eps1lon i just meant comparing the use of

const Avatar = React.forwardRef(function Avatar(props, ref) { ..
<div and things/>
}
export default withStyles(styles, {name: 'MuiAvatar'})(Avatar)

vs

function Avatatar(props){
const classes = useAvatarStyles()
<div and things/>
..} 

with makeStyles/hooks you don't need to wrap the component, meaning you don't need to worry about ref forwarding etc (which should make everything muuuch simpler)

as you can see in the picture i posted, the class name is still MuiAvatar without using a forward ref, just passing in the name option to makeStyles in the same was you do with withStyles

there is a huge benefit of doing this as well, in that you are really no longer coupled to react. the 'makeStyles' implementation is now basically a stylesheet which is framework agnostic

[eps1lon]

with makeStyles/hooks you don't need to wrap the component, meaning you don't need to worry about ref forwarding etc (which should make everything muuuch simpler)

Ref forwarding is not required for for withStyles or makeStyles. Your first example has the additional benefit that you can attach a ref to the Avatar. The second example does not allow this.

[jeremy-coleman]

Of course, but the second version doesnt use an hoc, so is forward ref even neccessary?

[eps1lon]

Of course, but the second version doesnt use an hoc, so is forward ref even neccessary?

I don't follow. The second example doesn't even use the forwardRef. I don't know if you need to forward the ref. That depends on your use case.

[jeremy-coleman]

It's a copy, literally copy and pasted copy, of the Avatar component from this library, not using an HOC and not using forwardRef, the picture is showing the MuiAvatar class still applies without using withStyles at all. So, if you need to create a downstream ref, the user can just create it now, because there are no wrappers. What im asking is..

-Hoc
--ForwardRef
---Avatar

equivalent to:
-Avatar(using makeStyles hook)

[oliviertassinari]

@jeremy-coleman I'm sorry. I don't understand your point regarding the ref. You have to use forwardRef anyway. The usage of makeStyles or withStyles or styled-components or emotion doesn't change a thing.

[oliviertassinari]

Anyway, I believe we have answered your question. There is no reason not to do it. We are investigating a migration to this pattern in #15023.

[jeremy-coleman]

Ahh ok , your take is just always wrap with a ref because there’s no reason not to? thanks Olivier. I rewrote pretty much the whole library using make styles, i can send a copy if you want. Its in typescript and no prop types though

[oliviertassinari]

@jeremy-coleman Oh wow, what was your motivation for the large enterprise? :)

[eps1lon]

withStyles or makeStyles have nothing to do with ref forwarding though. I still don't understand what the issue is. Ref forwarding is implemented a little bit different but both component patterns have this feature.

We need the ref internally and many of our users need it as well. We recommended the RootRef component for this earlier but that component uses findDOMNode which is more brittle than ref usage.

We want to switch to makeStyles eventually but you make it sound like this is just something we can do in a heartbeat. That's not how any of this works.

[jeremy-coleman]

@oliver bc things kept breaking, wrong types, themes not applying , multiple versions of jss, etc etc. i also wanted to take out prop types for dev. Prod build is much smaller, 500kb pre 70k post minify

[oliviertassinari]

@jeremy-coleman Do you think that some improvement could be bring back in the core of Material-UI?

[eps1lon]

@oliver bc things kept breaking, wrong types, themes not applying , multiple versions of jss, etc etc. i also wanted to take out prop types for dev. Prod build is much smaller, 500kb pre 70k post minify

@jeremy-coleman It seems like you're pretty frustrated which results in you making statements that aren't very constructive. I understand that this is a rant but please keep it civil.

That being sad let's try to unpack your statement:

things kept breaking

I would like to help you but I need specifics for those: What did you do? What did you expect? What happened instead?

wrong types

Types in the core are not coupled with the implementation. withStyles or makeStyles usage in the implementation does not change our types.

themes not applying

withStyles is just a hoc wrapper around makeStyles. It seems unlikely that the usage causes this breakage.

multiple versions of jss

This could happen in earlier alphas if you didn't follow our installation instructions for @material-ui/styles. Since withStyles uses makeStyles now there shouldn't be duplicate versions of jss anymore.

Prod build is much smaller, 500kb pre 70k post minify

This is self-evident: If you remove features your bundle will be smaller. Also 500k is probably post minify and 70k post compression. 70k before compression doesn't sound like it's bundling the full @material-ui/core bundle.

[eimanhakimy]

hello i'm new in material ui so can someone explain to me how css work by using withStyle and what a different between withStyle and MakeStyle..