mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.92k stars 32.27k forks source link

[Tabs] Invalid value warning triggered when using React.lazy #29209

Open iluvmemes opened 3 years ago

iluvmemes commented 3 years ago

v5.0.4

Much like the description in #8489, when using dynamic data to generate tabs and their contents, I receive the error:

MUI: The value provided to the Tabs component is invalid. The Tab with this value ("0") is not part of the document layout. Make sure the tab item is present in the document or that it's not display: none.

However, this only occurs once in our app after a hard refresh. I have reproduced this in a code sandbox however, it seems that there may be some caching with codesandbox.io that prevents it from happening.

In order to reproduce it in Codesandbox, you will need to add or remove a dependency such as lodash or something from the fork.

https://codesandbox.io/s/sparkling-grass-sfidi?file=/src/TabIssue.jsx

michaldudak commented 3 years ago

@iluvmemes would you be able to create a reproduction in a repo I could investigate locally? Having to add dependencies to see the error makes it somewhat hard to investigate.

vicentedr96 commented 3 years ago

I have the same problem

remyzane commented 3 years ago

same problem while import use React.lazy, e.g.

const Xyz = React.lazy(()=>import('...')

error: <Xyz />

fix: <Suspense fallback={null}><Xyz /></Suspense>

iluvmemes commented 3 years ago

same problem while import use React.lazy, e.g.

const Xyz = React.lazy(()=>import('...')

error: <Xyz />

fix: <Suspense fallback={null}><Xyz /></Suspense>

I've been trying to get a repo together to reproduce the problem per @michaldudak's request with lazy loading (we use that in our app). @remyzane we're already using suspense in our app and still receive the warnings. ☹️

iluvmemes commented 3 years ago

@remyzane was correct. It looks as though it happens when the contents of the actual tab is lazy loaded. @michaldudak here is a repo you can look at.

https://github.com/iluvmemes/MUI-29209

michaldudak commented 3 years ago

This warning is caused by the code added as a fix for #26582. @oliviertassinari you suggested to check if the tab width and height are 0. This, unfortunately, captures the case described here: when React.lazy is used, the tab's width and height are 0. I propose to change the condition this way:

--- a/packages/mui-material/src/Tabs/Tabs.js
+++ b/packages/mui-material/src/Tabs/Tabs.js
@@ -357,9 +357,7 @@ const Tabs = React.forwardRef(function Tabs(inProps, ref) {
           if (
             process.env.NODE_ENV !== 'test' &&
             !warnedOnceTabPresent &&
-            tabMeta &&
-            tabMeta.width === 0 &&
-            tabMeta.height === 0
+            tab?.style?.display === 'none'
           ) {
             tabsMeta = null;
             console.error(

It should cover the original case described in #26582 and prevent the warning from firing in this case.

oliviertassinari commented 3 years ago

The regression in the codesandbox reproduction started between v5.0.0-beta.0 and v5.0.0-rc.0. What we did is the release of #27791. I assume that codesandbox triggers a layout change, that triggers the resize observer, that trigger handleResize when the DOM notes are unmounted.

oliviertassinari commented 3 years ago

Regarding looking at tab?.style?.display === 'none', if we do this, I personally don't think that it would make sense to keep the warning anymore. A tab can be outside of the DOM layout for many different reasons. Instead, we could either remove the warning or ignore this issue.

oliviertassinari commented 3 years ago

when React.lazy is used, the tab's width and height are 0.

Does it mean that the tab underline indicator is at the wrong position?

I have given it a try: yes. But thanks to #27791, it update it at the right position.

oliviertassinari commented 3 years ago

Ok, as far as I can dive into this problem, this is a bug with React & Suspense. I cloned https://github.com/iluvmemes/MUI-29209, I added the Tabs.js locally and added a breakpoint when the warning triggers.

When useEffect/useLayoutEffect run inside the Tabs, React still has display: none applied everywhere:

Screenshot 2021-11-03 at 13 36 32

It reminds me: https://github.com/facebook/react/issues/14536.

oliviertassinari commented 3 years ago

Ok, looks like it's a known bug with React 17.0.2 @eps1lon was already into it https://github.com/facebook/react/issues/21510. I think that we can close this issue with the "external dependency" label. It supposed to be fixed in React 18: https://github.com/facebook/react/issues/21510#issuecomment-861955777.

eps1lon commented 3 years ago

Duplicate of https://github.com/mui-org/material-ui/issues/14077

mikkopori commented 2 years ago

For me this issue is still happening even after upgrading to "react": "^18.2.0", so could be something else as well?

BryanAPEX commented 2 years ago

I'm also still experiencing this after upgrading to React 18.2.0 and MUI 5.5.3.

michaldudak commented 2 years ago

@mikkopori, @BryanAPEX thanks for letting us know. I'm reopening the issue.