Closed Technologeek closed 11 months ago
@Technologeek Thanks for opening this issue, this is related to #17070 and how we should improve our testing guide. I think that it's a topic we have already discussed in #16523. The solution we went for is to test all our components with:
import ReactTestRenderer from 'react-test-renderer';
ReactTestRenderer.act(() => {
ReactTestRenderer.create(<Foo />, {
createNodeMock: node => {
return document.createElement(node.type);
},
});
});
+80% of our components are passing this test. We will work on raising it to 100%. The ones that don't are the components using react-transition-groups or mounting a Portal. It's not the case of the tabs.
I assume that you are using @storybook/addon-storyshots
: https://storybook.js.org/docs/testing/structural-testing/, https://github.com/storybookjs/storybook/tree/master/addons/storyshots/storyshots-core.
Looking at the sources, it leads me to: https://github.com/storybookjs/storybook/blob/v5.2.0-beta.39/addons/storyshots/storyshots-core/src/frameworks/react/renderTree.js.
Alright, it seems that part of the solution is documented in: https://github.com/storybookjs/storybook/tree/master/addons/storyshots/storyshots-core#using-createnodemock-to-mock-refs.
Could you try the following?
import ReactTestRenderer from 'react-test-renderer';
import initStoryshots, { snapshotWithOptions } from '@storybook/addon-storyshots';
initStoryshots({
test: snapshotWithOptions({
renderer: storyElement =>
ReactTestRenderer.act(() => {
ReactTestRenderer.create(storyElement, {
createNodeMock: node => document.createElement(node.type),
});
}),
}),
});
or
import initStoryshots, { snapshotWithOptions } from '@storybook/addon-storyshots'
initStoryshots({
test: snapshotWithOptions({
createNodeMock: node => document.createElement(node.type),
}),
})
@oliviertassinari Thanks a lot for your quick reply! As far as my use-case is concerned, I'm not getting any console-errors or warnings and the component seems to pass all other tests. I could apply your suggestion but it is not needed at the moment. We have multiple such third party elements which fail tests due to ForwardRefs but the component works great without any console errors. For now, I prefer to just ignore that story. If it helps anybody, this is how you ignore a story..
//In your storyshots.test.js file
import initStoryshots from "@storybook/addon-storyshots";
initStoryshots({
storyKindRegex: /^((?!.*?NameOfYourStory).)*$/
});
Thanks again for your help!
Hey @oliviertassinari just wanted to say that the solution you provided with act
and createNodeMock
worked and helped out with our tests
Thanks 🙏
@carloscuesta Thanks for the feedback! Now, we need to update the testing guide: #17070. Any help would be appreciated.
this is also happening when using the unstable_createMuiTheme
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 `Transition`.
in Content (created by Transition)
in Transition (created by ForwardRef(Grow))
in ForwardRef(Grow) (created by ForwardRef(StrictModeGrow))
in ForwardRef(StrictModeGrow) (created by ForwardRef(Snackbar))
in div (created by ClickAwayListener)
in ClickAwayListener (created by ForwardRef(Snackbar))
in ForwardRef(Snackbar) (created by WithStyles(ForwardRef(Snackbar)))
in WithStyles(ForwardRef(Snackbar)) (created by Snackbar___c2)
in Snackbar___c2 (created by Snackbar)
Uncaught TypeError: Cannot read property 'scrollTop' of null
at reflow (utils.js:2)
at Object.handleEnter [as onEnter] (Grow.js:48)
at Transition.performEnter (Transition.js:260)
at Transition.updateStatus (Transition.js:231)
at Transition.componentDidMount (Transition.js:174)
this is also happening when using the unstable_createMuiTheme
If you use this theme you need to make sure that children of transition components forward their refs to the outermost DOM node. The original plan was to add documentation for this but there were other StrictMode issues that weren't fixable so I didn't pursue this beyond internal usage.
Would greatly appreciate it if someone would add docs for unstable_createMuiTheme
with caveats to https://material-ui.com/customization/theming:
thanks for this explanation, now I know how to fix this
I'm using it only in Dev to test how it goes
import initStoryshots, { snapshotWithOptions } from '@storybook/addon-storyshots' initStoryshots({ test: snapshotWithOptions({ createNodeMock: node => document.createElement(node.type), }), })
This works fine for me. Thank you!
import initStoryshots, { snapshotWithOptions } from '@storybook/addon-storyshots' initStoryshots({ test: snapshotWithOptions({ createNodeMock: node => document.createElement(node.type), }), })
This works fine for me. Thank you!
I'm not really sure I understand what the internal problem is, but the code above didn't work for me.
It's crashing on the Zoom
component:
const transitionDuration = {
enter: theme.transitions.duration.enteringScreen,
exit: theme.transitions.duration.leavingScreen,
};
<Zoom in={true} timeout={transitionDuration} unmountOnExit>
this is also happening when using the unstable_createMuiTheme
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 `Transition`. in Content (created by Transition) in Transition (created by ForwardRef(Grow)) in ForwardRef(Grow) (created by ForwardRef(StrictModeGrow)) in ForwardRef(StrictModeGrow) (created by ForwardRef(Snackbar)) in div (created by ClickAwayListener) in ClickAwayListener (created by ForwardRef(Snackbar)) in ForwardRef(Snackbar) (created by WithStyles(ForwardRef(Snackbar))) in WithStyles(ForwardRef(Snackbar)) (created by Snackbar___c2) in Snackbar___c2 (created by Snackbar)
Uncaught TypeError: Cannot read property 'scrollTop' of null at reflow (utils.js:2) at Object.handleEnter [as onEnter] (Grow.js:48) at Transition.performEnter (Transition.js:260) at Transition.updateStatus (Transition.js:231) at Transition.componentDidMount (Transition.js:174)
Same happened to me after upgrading to v5.0.0-alpha when I use createMuiTheme
this way:
createMuiTheme({
components: {
MuiPaper: {
defaultProps: { component: ForwardedRefComponent },
},
},
});
Later, some of my Menu
elements crash with exactly same error as quoted.
I really don't understand why this occurs, as I DO use forwardRef
for component
prop, but the error still occurs.
@mui-org/core I found this error when testing codemod migration with material-ui-store and takes a while to figure out what's wrong. Here is what I think we can help making migration smoother
Transition
(no transition) and display warning that the child should have forwardRefwhich options do you think it is worth to do?
@siriwatknp This issue is about the lack of ref in the test environment (react-test-renderer). I think that the issue you are talking about is related but different. I'm not sure we should discuss it here. Maybe it should happen in a new issue with a reproduction?
@siriwatknp This issue is about the lack of ref in the test environment (react-test-renderer). I think that the issue you are talking about is related but different. I'm not sure we should discuss it here. Maybe it should happen in a new issue with a reproduction?
Okay, will create a new issue with reproduction: #27154.
For me, issue occurs even in browser. It's very rare so I'm not able to reproduce. Probably happens after the Tabs
are unmounted, which means that not all calls to updateScrollButtonState()
are cleared properly. Also updateScrollButtonState
is provided as callback in imperative handler so with calling action.updateScrollButtons()
after lifecycle ended you can easily shoot yourself in the foot.
TypeError
Cannot destructure property 'scrollTop' of 'H.current' as it is null.
https://sentry.io/share/issue/8026280039644d4f9a4784928818290f/
Could we check that tabsRef.current !== null
in the beginning of updateScrollButtonState()
function? So that calling updateScrollButtonState()
would be safe.
For me, issue occurs even in browser. It's very rare so I'm not able to reproduce. Probably happens after the
Tabs
are unmounted, which means that not all calls toupdateScrollButtonState()
are cleared properly. AlsoupdateScrollButtonState
is provided as callback in imperative handler so with callingaction.updateScrollButtons()
after lifecycle ended you can easily shoot yourself in the foot.TypeError Cannot destructure property 'scrollTop' of 'H.current' as it is null.
https://sentry.io/share/issue/8026280039644d4f9a4784928818290f/
Could we check that
tabsRef.current !== null
in the beginning ofupdateScrollButtonState()
function? So that callingupdateScrollButtonState()
would be safe.
Yes, I could detect it too on Sentry. It happens on Chrome Browser at least:
Cannot destructure property 'scrollTop' of 'be.current' as it is null.
I suspect the Tabs component too.
Could we check that tabsRef.current !== null in the beginning of updateScrollButtonState() function? So that calling updateScrollButtonState() would be safe.
Feel free to submit a PR.
Observed. ☝️ attaching context while debugging in browser:
TypeError: Cannot destructure property 'scrollTop' of 'tabsRef.current' as it is null.
at http://localhost:8001/js/vendors-node_modules_mui_icons-material_CheckBox_js-node_modules_mui_icons-material_CheckBoxO-6c59ee.570e64915171c9efc4ec.js:1224:9
at http://localhost:8001/js/app.645260a71ad0be185387.js:18951:19
at ResizeObserver.<anonymous> (http://localhost:8001/js/vendors-node_modules_mui_icons-material_CheckBox_js-node_modules_mui_icons-material_CheckBoxO-6c59ee.570e64915171c9efc4ec.js:1254:7)
at later (http://localhost:8001/js/app.645260a71ad0be185387.js:18162:12)
set <Tabs />
component property scrollButtons={false}
.
It disables the following:
const updateScrollButtonState = useEventCallback(() => {
if (scrollable && scrollButtons !== false) {
const {
scrollTop,
scrollHeight,
clientHeight,
scrollWidth,
clientWidth
} = tabsRef.current;
let showStartScroll;
let showEndScroll;
if (vertical) {
showStartScroll = scrollTop > 1;
showEndScroll = scrollTop < scrollHeight - clientHeight - 1;
} else {
const scrollLeft = getNormalizedScrollLeft(tabsRef.current, theme.direction); // use 1 for the potential rounding error with browser zooms.
showStartScroll = isRtl ? scrollLeft < scrollWidth - clientWidth - 1 : scrollLeft > 1;
showEndScroll = !isRtl ? scrollLeft < scrollWidth - clientWidth - 1 : scrollLeft > 1;
}
if (showStartScroll !== displayScroll.start || showEndScroll !== displayScroll.end) {
setDisplayScroll({
start: showStartScroll,
end: showEndScroll
});
}
}
});
With the switch to the Intersection Observer API for handling scroll button visibility since @mui/material
version 5.14.2
, I believe the error should no longer occur.
Closing this as completed since the error should no longer occur 😅 but we can always reopen it if new reports come in.
Current Behavior 😯
Hi, I'm using the Tabs component and passing all the props that I require using my own wrapper component. Everything is working fine except when I add scrollable prop to the component.
This works as it's supposed to be without any errors on the console but when I run tests through Jest of updating the snap-shots using storybook, I'm getting an error.
Error: Uncaught [TypeError: Cannot read property 'offsetHeight' of null]
Error: Uncaught [TypeError: Cannot read property 'scrollTop' of null]
Expected Behavior 🤔
The tests should pass just like it does when the scrollable prop is not passed on the Tabs component.
Steps to Reproduce 🕹
Steps:
Context 🔦
Your Environment 🌎