material-ui
material-ui copied to clipboard
[material-ui][Tabs] Scrollable variant fails test when clicking the tab (error reading 'offsetHeight')
Steps to reproduce
Update Jun 7, 2024: There was a comment on the PR questioning whether the original live example isn't depending on https://github.com/mui/material-ui/pull/36485. Therefore, a new live example was created and linked below, which is connected to https://github.com/nicholeuf/mui-test-fixture-41388.
Original link to live example: codesandbox.io
This only seems to happen under test. See the test scenario in the link above.
Current behavior
The test fails with the following error:
TypeError: Cannot read properties of null (reading 'offsetHeight')
at setMeasurements (node_modules/.pnpm/@[email protected]_@[email protected]_@[email protected]_@[email protected][email protected][email protected]/node_modules/@mui/material/node/Tabs/ScrollbarSize.js:40:47)
It fails at:
https://github.com/mui/material-ui/blob/2ef685d19ec53aafcddf15f67364a4974a86eaf2/packages/mui-material/src/Tabs/ScrollbarSize.js#L25-L27
Expected behavior
The test passes.
Context
I would like to test my application with a similar code setup.
Your environment
npx @mui/envinfo
From live example:
System:
OS: Linux 6.1 Ubuntu 20.04.6 LTS (Focal Fossa)
Binaries:
Node: 20.12.1 - /home/codespace/nvm/current/bin/node
npm: 10.5.0 - /home/codespace/nvm/current/bin/npm
pnpm: 8.15.6 - /home/codespace/nvm/current/bin/pnpm
Browsers:
Chrome: Not Found
npmPackages:
@emotion/react: ^11.11.4 => 11.11.4
@emotion/styled: ^11.11.5 => 11.11.5
@mui/material: ^5.15.19 => 5.15.19
@mui/material-nextjs: ^5.15.11 => 5.15.11
@types/react: ^18.3.3 => 18.3.3
react: ^18.3.1 => 18.3.1
react-dom: ^18.3.1 => 18.3.1
typescript: ^5.4.5 => 5.4.5
From original project where error was discovered:
System:
OS: macOS 14.3.1
Binaries:
Node: 18.19.1 - ~/.nvm/versions/node/v18.19.1/bin/node
npm: 10.2.4 - ~/.nvm/versions/node/v18.19.1/bin/npm
pnpm: Not Found
Browsers:
Chrome: 122.0.6261.111
Edge: Not Found
Safari: 17.3.1
npmPackages:
@emotion/react: ^11.11.4 => 11.11.4
@emotion/styled: ^11.11.0 => 11.11.0
@mui/base: 5.0.0-beta.38
@mui/core-downloads-tracker: 5.15.12
@mui/icons-material: ^5.15.12 => 5.15.12
@mui/material: ^5.15.12 => 5.15.12
@mui/material-nextjs: ^5.15.11 => 5.15.11
@mui/private-theming: 5.15.12
@mui/styled-engine: 5.15.11
@mui/system: 5.15.12
@mui/types: 7.2.13
@mui/utils: 5.15.12
@types/react: ^18.2.63 => 18.2.63
react: ^18.2.0 => 18.2.0
react-dom: ^18.2.0 => 18.2.0
typescript: ^5.3.3 => 5.3.3
Search keywords: tabs scrollable test
Same here on Next.js with ppr activated (https://nextjs.org/docs/app/api-reference/next-config-js/partial-prerendering).
Looks like a bug.
Same happens for me but without tests, just when trying to navigate to a specific view.
EDIT: What fixed it for me is using react and react-dom version 18.2.0 instead of 0.0.0-experimental-fb10a2c66-20240228
will be this fixed? can not update anymore nextjs , this is a simple check if the reference is undefined .....
This fails for me using the latest React 19 RC and React Compiler.
Also started getting this issue when rendering Tabs
with React 19 / React Compiler. Monkey-patching setMeasurements
inside @mui/material/Tabs/ScrollbarSize.js
to the version below helps unblock development, hope this gets patched as React Compiler is a game changer for our app's performance.
const setMeasurements = () => {
if (nodeRef.current && scrollbarHeight.current) {
scrollbarHeight.current = nodeRef.current.offsetHeight - nodeRef.current.clientHeight;
}
};
In case anyone else is blocked by this try https://www.npmjs.com/package/patch-package until this gets fixed
Feel free to create a PR with a reproduction and a test.
Feel free to create a PR with a reproduction and a test.
@ZeeshanTamboli I created a PR https://github.com/mui/material-ui/pull/42512 and linked it to this issue
This only seems to happen under test. See the test scenario in the link above.
From the reproduction https://github.com/nicholeuf/mui-test-fixture-41388, it seems to me that it's only that the snapshot test env isn't setup correctly. Doing this change fixes the issue:
diff --git a/utils/test-utils.tsx b/utils/test-utils.tsx
index f27a491..51c302a 100644
--- a/utils/test-utils.tsx
+++ b/utils/test-utils.tsx
@@ -17,8 +17,18 @@ const customRender = (
options?: Omit<RenderOptions, 'wrapper'>
) => render(ui, { wrapper: TestWrapper, ...options });
-const customSnapshotRender = (children: ReactElement) =>
- renderer.create(<TestWrapper>{children}</TestWrapper>);
+
+const customSnapshotRender = (children: ReactElement) => {
+ let root;
+ renderer.act(() => {
+ root = renderer.create(<TestWrapper>{children}</TestWrapper>, {
+ createNodeMock: (node) => {
+ return document.createElement(node.type as keyof HTMLElementTagNameMap);
+ },
+ });
+ });
+ return root;
+}
export * from '@testing-library/react';
export { customRender as render, customSnapshotRender as renderSnapshot };
See https://github.com/mui/material-ui/pull/16523/files#diff-d0f975a526b07952a6902336247e87b2de9a2d1ecd6c411c7492ac06009c8c73 for historical context.
This fails for me using the latest React 19 RC
A reproduction of the issue: https://github.com/oliviertassinari/material-ui-issue-41388 issue with React 19.0.0-rc.0 and Next.js v15.0.0-rc.0.
Looking closer, it's an issue with the ref. I have linked to #42381. #42512 is definitely the wrong fix.
Here is what happens:
- Emotion adds a ref: https://github.com/emotion-js/emotion/blob/4cc565f1b7a576902b6360654afa4ce176698f76/packages/styled/src/base.js#L160
- Since v19, React forwards it to the props: https://github.com/mui/material-ui/blob/2ef685d19ec53aafcddf15f67364a4974a86eaf2/packages/mui-material/src/Tabs/ScrollbarSize.js#L21
- Once the ref of Emotion lands on the DOM element, it overrides the logic of the component https://github.com/mui/material-ui/blob/2ef685d19ec53aafcddf15f67364a4974a86eaf2/packages/mui-material/src/Tabs/ScrollbarSize.js#L52
To fix this, we can:
- In Material UI's
<ScrollbarSize>
internal component:
-return <div style={styles} ref={nodeRef} {...other} />;
+return <div style={styles} {...other} ref={nodeRef} />;
but this feels like a hack
- In Emotion, which would make more sense to me, in https://github.com/emotion-js/emotion/blob/4cc565f1b7a576902b6360654afa4ce176698f76/packages/styled/src/base.js#L160, only forward a ref, if one is defined.
+ if (ref) {
newProps.ref = ref;
+ }
cc @Andarist
Issue created https://github.com/emotion-js/emotion/issues/3204.
Ok, the issue was solved in Emotion (not released yet): https://github.com/emotion-js/emotion/issues/3204
But styled-components has the same problem 🙈: https://github.com/styled-components/styled-components/issues/4331 (which we need to support for the sc engine adapter)
I think we can make the change in Material UI too, and call it a day. It will take some time for the developers to get the fix from Emotion, I'm not even sure Styled-components is maintained anymore, anyway.
-return <div style={styles} ref={nodeRef} {...other} />;
+return <div style={styles} {...other} ref={nodeRef} />;
To be noted that it only impacts this component because it doesn't use React.forwardRef
.
Any update on this, please?
@oliviertassinari looks like they fixed this in version 11.12.0
just as a head up! Thank you for making the fix.