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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Tooltip] Error message from using a class component #33152

Closed
2 tasks done
joshkel opened this issue Jun 14, 2022 · 4 comments 路 Fixed by #33325
Closed
2 tasks done

[Tooltip] Error message from using a class component #33152

joshkel opened this issue Jun 14, 2022 · 4 comments 路 Fixed by #33325
Labels
component: tooltip This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation

Comments

@joshkel
Copy link
Contributor

joshkel commented Jun 14, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

If a React class component is used as the child of an MUI Tooltip, you get an exception with a somewhat cryptic error message. In Chrome:

Cannot read properties of undefined (reading 'addEventListener')
    at prepare (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/useIsFocusVisible.js:57:7

In Firefox:

doc is undefined
prepare@https://3v1d9x.csb.app/node_modules/
mui/utils/esm/useIsFocusVisible.js:57:3
Full stack trace in Chrome
TypeError
Cannot read properties of undefined (reading 'addEventListener')
    at prepare (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/useIsFocusVisible.js:57:7
    at eval (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/useIsFocusVisible.js:80:7
    at setRef (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/setRef.js:7:5
    at eval (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/useForkRef.js:15:26
    at setRef (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/setRef.js:7:5
    at eval (https://3v1d9x.csb.app/node_modules/
mui/utils/esm/useForkRef.js:16:26
commitAttachRef
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:23543:20
commitLayoutEffectOnFiber
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:23401:9
commitLayoutMountEffects_complete
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:24578:9
commitLayoutEffects_begin
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:24564:7
commitLayoutEffects
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:24502:3
commitRootImpl
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:26779:5
commitRoot
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:26638:5
finishConcurrentRender
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:25937:9
performConcurrentWorkOnRoot
https://3v1d9x.csb.app/node_modules/react-dom/cjs/react-dom.development.js:25765:7
workLoop
https://3v1d9x.csb.app/node_modules/scheduler/cjs/scheduler.development.js:266:34
flushWork
https://3v1d9x.csb.app/node_modules/scheduler/cjs/scheduler.development.js:239:14
performWorkUntilDeadline
https://3v1d9x.csb.app/node_modules/scheduler/cjs/scheduler.development.js:533:21
(anonymous function)
https://codesandbox.io/static/js/vendors~react-devtools-backend.759c29504.chunk.js:1:230839
u
https://codesandbox.io/static/js/vendors~react-devtools-backend.759c29504.chunk.js:1:230954
n
https://codesandbox.io/static/js/vendors~react-devtools-backend.759c29504.chunk.js:1:229899

Expected behavior 馃

It works, or a clean error message is shown.

Steps to reproduce 馃暪

Steps: See https://codesandbox.io/s/happy-sun-3v1d9x?file=/src/App.tsx

Context 馃敠

This issue occurs because Tooltip (and, perhaps, similar MUI components) require a ref to an HTML element, not a React class component.

Calling findDOMNode on node within useIsFocusVisible should work, but findDOMNode is deprecated.

The solution for affected code is to forward the ref to the underlying DOM node, as described at https://thewebdev.info/2022/04/28/how-to-use-react-forwardref-in-a-class-based-component/.

Can/should useIsFocusVisible check for class components in development mode and throw a developer-friendly error message?

    if (node != null) {
      if (process.env.NODE_ENV !== 'production') {
        if (!(node instanceof HTMLElement)) {
          throw new Error('MUI: The `children` component passed to this MUI component is not an HTML element. ' +
              'If using a class component, please change it to forward the ref to its DOM node.');
        }
      }
      prepare(node.ownerDocument);
    }

Your environment 馃寧

npx @mui/envinfo
  System:
    OS: macOS 12.4
  Binaries:
    Node: 14.19.1 - ~/.nvm/versions/node/v14.19.1/bin/node
    Yarn: 3.2.1 - /opt/homebrew/bin/yarn
    npm: 6.14.16 - ~/.nvm/versions/node/v14.19.1/bin/npm
  Browsers:
    Chrome: 102.0.5005.115
    Edge: Not Found
    Firefox: 101.0.1
    Safari: 15.5
  npmPackages:
    @emotion/react:  11.9.0
    @emotion/styled:  11.8.1
    @mui/base:  5.0.0-alpha.82
    @mui/icons-material:  5.8.0
    @mui/lab:  5.0.0-alpha.83
    @mui/material:  5.8.1
    @mui/private-theming:  5.8.0
    @mui/styled-engine:  5.8.0
    @mui/system:  5.8.1
    @mui/types:  7.1.3
    @mui/utils:  5.8.0
    @mui/x-date-pickers:  5.0.0-alpha.1
    @types/react: ^17.0.45 => 17.0.45
    react: ^17.0.2 => 17.0.2
    react-dom: 17.0.2 => 17.0.2
    typescript: ^4.7.2 => 4.7.2```
</details>
@joshkel joshkel added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jun 14, 2022
@michaldudak
Copy link
Member

Non-ref forwarding class components are not supported by the Tooltip (#21811).
We don't make it clear in the docs, though.

@michaldudak michaldudak added docs Improvements or additions to the documentation component: tooltip This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jun 28, 2022
@michaldudak michaldudak changed the title Error message from using a class component with Tooltip [Tooltip] Error message from using a class component Jun 28, 2022
@joshkel
Copy link
Contributor Author

joshkel commented Jun 28, 2022

@michaldudak Thank you. I had figured that out and was able to work around it, but I also wondered if MUI's useIsFocusVisible should include a development-time check for non-class components to make this clearer. (I had tried to include these details in my original issue but botched the Markdown+HTML, so it wasn't viewable - it's fixed now, so please take a look.)

@michaldudak
Copy link
Member

Yes, that's a good idea. I'd check for Element instead of HTMLElement, though, as SVGs elements should work just fine with the Tooltip. Would you like to create a PR?

@joshkel
Copy link
Contributor Author

joshkel commented Jun 28, 2022

@michaldudak Done; see #33323.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: tooltip This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants