Material-ui: Menu Component "Function components cannot be given refs"

[Hens94]

• [x] This is not a v0.x issue.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

When trying to set the anchorEl to a Menu component, I expect it to work fine without warning

Current Behavior 😯

Warning: Function components cannot be given refs. Attempts to access this ref will fail. Did you mean to use React.forwardRef()?

Check the render method of ForwardRef(Menu).

Steps to Reproduce 🕹

Link: https://codesandbox.io/embed/createreactapp-bnnxl
the warning appear when click it the Home Icon

Your Environment 🌎

| Tech | Version |
|--------------|---------|
| Material-UI | v4.0.1 |
| React | 16.8.6 |
| Browser | Chrome v74 |

[oliviertassinari]

@Hens94 Please provide a full reproduction test case. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template _may_ be a good starting point. Thank you!

[Hens94]

@oliviertassinari
this is the code with a full reproduction: https://codesandbox.io/embed/createreactapp-bnnxl
the warning appear when click it the Home Icon

[oliviertassinari]

This is an issue with the NavLink component. This react-router component should forward its ref, it doesn't yet: https://github.com/ReactTraining/react-router/issues/6056#issuecomment-435524678. I hope https://material-ui.com/guides/composition/#react-router-demo helps to explain why in more details.

I have noticed another issue; you are using the ul > a > li DOM structure. This is not encouraged.
You should consider merging the two.

import React from "react";
import { NavLink } from "react-router-dom";
import PropTypes from "prop-types";
import Menu from "@material-ui/core/Menu";
import MenuItem from "@material-ui/core/MenuItem";
import ListItemIcon from "@material-ui/core/ListItemIcon";
import ListItemText from "@material-ui/core/ListItemText";
import Icon from "@material-ui/core/Icon";

const ForwardNavLink = React.forwardRef((props, ref) => (
  <NavLink {...props} innerRef={ref} />
));

export default const PopupItem = ({ currentTarget, childrenMenu, closePopup }) => (
  <Menu
    anchorEl={currentTarget}
    open={!!currentTarget}
    onClose={closePopup}
  >
    {childrenMenu.map((val, index) => {
      const {
        text: textChildren,
        icon: iconChildren,
        link: linkChildren
      } = val;

      return (
        <MenuItem
          key={`MPU-${index}-${textChildren}`}
          onClick={closePopup}
          className="nested"
          style={{ color: "inherit" }}
          to={linkChildren}
          exact
          activeClassName="active-menu"
          style={{ textDecoration: "none", color: "inherit" }}
          component={ForwardNavLink}
        >
          <ListItemIcon>
            <Icon className="icon material-icons-round">{iconChildren}</Icon>
          </ListItemIcon>
          <ListItemText primary={textChildren} className="text" />
        </MenuItem>
      );
    })}
  </Menu>
);

https://codesandbox.io/s/createreactapp-x6u1h

[jedwards1211]

Is there a way for MUI to provide a more helpful error message for this? I was baffled by https://github.com/jcoreio/material-ui-popup-state/issues/11 for awhile until I dug into Menu.js and noticed that it's injecting something refs into its children. The error is very non-obvious since the userland code doesn't look like it's using any refs at all.

[jedwards1211]

Also I'm passing getContentAnchorEl={null} as an override to <Menu> so FWIW, the refs it injects into the children are not serving any purpose, and maybe it should avoid injecting them if it sees that getContentAnchorEl has been overridden?

[jedwards1211]

Also on a broader scale -- I think it would be more convenient for everyone if React automatically forwards all refs to the first DOM node (forwardRef would only be needed to indicate which child of a React.Fragment the ref gets forwarded to) and drops support for refs to component instances (there are alternatives). Do you agree? I don't have the clout to advocate this to the React team but I'm sure you do.
Ref forwarding is currently my least favorite hassle working with React.

[joshwooding]

@jedwards1211 Pretty sure there are already plans to do automatic forwarding in React. I can’t remember where I read it though.

[eps1lon]

@jedwards1211 Please open a new issue and fill out the template. We already have additional warnings for function components been given refs. If there are instances where this doesn't happen I'd like to add it.

[waleedshkt]

I found a simple solution.

Just wrap the child functional component with a simple div.

[chen406226]

I found a simple solution.

Just wrap the child functional component with a simple div.

TKS haha