Menu Component "Function components cannot be given refs"

[Hens94]

• This is not a v0.x issue.
• 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: remix-run/react-router#6056 (comment). 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 jcoreio/material-ui-popup-state#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.

Wrap the child functional component with a <div>.

[TodBob]

@waleedshkt Ty for solution :-D :-D
But still will be nice to have real solution in MUI.

Im using MUI v4 and <Menu /> component with <MenuItem> component. I got exactly same error as is in title.
In my app I moved <MenuItem> to separate file as reusable component. Then importing and using as usual in react.
Solution was moved every <CUSTOMMenuItem> component instance inside div. Which is nice, it works but creates another unnecessarily divs.

<Menu
  keepMounted
  open={contextMenu !== null}
  onClose={() => handleClose(setContextMenu)}
  anchorReference="anchorPosition"
  anchorPosition={
  (contextMenu !== null)
    ? { top: contextMenu.mouseY, left: contextMenu.mouseX }
    : undefined
  }
>
  <div>
    <ContextMenuItem
      setState={setContextMenu}
      valueText="Example"
    >
      <SendIcon fontSize="small" />
    </ContextMenuItem>
  </div>
  <div>
    <ContextMenuItem
      setState={setContextMenu}
      valueText="Delete panel"
      customFunc={() => removeAllPanelDataSaga({ panelId })}
    >
      <DeleteForeverIcon fontSize="small" />
    </ContextMenuItem>
  </div>
</Menu>

[Hiroki111]

I saw a similar (probably the same) issue with @material-ui/core/Fade.
@waleedshkt 's approach solved it.

i.e.

// "Function components cannot be given refs" error message in console
<Fade in={open}>
    <MyModalComponent handleClose={handleClose} />
</Fade>

// The message is gone
<Fade in={open}>
  <div>
    <MyModalComponent handleClose={handleClose} />
  </div>
</Fade>

[PRossetti]

I found a simple solution.

Wrap the child functional component with a <div>.

Thank you! This solved my problem. I had this breaking my application:

import { Snackbar } from '@material-ui/core'
import MuiAlert, { AlertProps } from '@material-ui/lab/Alert'

...
function Alert(props: AlertProps) {
  return <MuiAlert elevation={6} variant="filled" {...props} />
}

...
<Snackbar {...}>
<Alert>Some text</Alert>
</Snackbar>

Changed the last part to this:

...
<Snackbar {...}>
<div><Alert {...}>Some text</Alert></div>
</Snackbar>

And now it works like a charm! Thank you again!

[trungduchoang]

Just like @TomasKomprs But add 1 div between <Menu> and its imported childrens is enough

function Any() {
  return (
    <Menu>
      <div>
        <AddPageLayoutBlock />
        <TestBtn />
      </div>
    </Menu>
  );
}

function Any({ children }) {
  return (
    <Menu>
      <div>{children}</div>
    </Menu>
  );
}