TextField multiline with no rows throws error when using ReactTestRenderer
#16491
Comments
|
@dondi Thanks for the report, the issue is that the input ref is null when the useLayoutEffect effect triggers. I'm wondering if we shouldn't change the logic to: diff --git a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js
index 4a0cf1d99..a956ed7e9 100644
--- a/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js
+++ b/packages/material-ui/src/TextareaAutosize/TextareaAutosize.js
@@ -7,7 +7,10 @@ function getStyleValue(computedStyle, property) {
return parseInt(computedStyle[property], 10) || 0;
}
-const useEnhancedEffect = typeof window !== 'undefined' ? React.useLayoutEffect : React.useEffect;
+const useEnhancedEffect =
+ typeof window !== 'undefined' && process.env.NODE_ENV !== 'test'
+ ? React.useLayoutEffect
+ : React.useEffect;
const styles = {
/* Styles applied to the shadow textarea element. */```Reference: |
|
In v1 -> v3 we were avoiding the issue with: |
|
@dondi What do you think? Do you want to submit a pull request? I have run the following script: import React from "react";
import ReactTestRenderer from "react-test-renderer";
import * as core from '@material-ui/core';
// Throw when accessing the children
const broken = [
'WithStyles(ForwardRef(ExpansionPanel))',
'WithStyles(WithFormControlContext(ForwardRef(FormControlLabel)))',
'WithStyles(Tooltip)',
]
Object.keys(core).forEach(key => {
const Component = core[key];
if (!Component.displayName || broken.indexOf(Component.displayName) !== -1) {
return;
}
it(`should be OK with ${Component.displayName}`, () => {
ReactTestRenderer.create(<Component />);
});
})
The problem seems to be isolated to the TextareaAutosize :) |
oliviertassinari
added
test
good first issue
|
Thanks for the quick analysis! It'll take a little longer to digest everything but sure, once I’ve gotten myself situated, I can submit something. |
|
@oliviertassinari I have tried the two approaches, #16491 (comment) and #16491 (comment), and the first approach which swaps out The second approach, which just returns if the ref is null, passes the test case. I think I’ll go with that, although I don’t know the code base well enough to know whether just returning from I guess we’ll find out when the PR goes up for review 🙂 I just thought I would comment ahead of this in order to explain why the PR will look the way it does. |
@dondi Thanks for trying it out. However, I was proposing the oppossite. Adding an env test for test should make the logic uses useEffect instead of useLayoutEffect. Could you try again :) ? |
|
@oliviertassinari I tried again (and copy-pasted the code from the comment to be sure) but still get an error: I’ll dig in a bit more to see what’s up. The discussion here and in the PR is giving me a better big-picture sense of what’s going on and so I’m getting some other ideas now. |
|
Just stumbled upon this, my solution for this was to not use react-test-renderer, but Shoutout to https://www.leighhalliday.com/introduction-react-testing-library where I was able to find this API for generating snapshots. |
Expected Behavior 🤔
When a renderer for
<TextField multiline />is created byReactTestRenderer.create, no errors should be thrown:Current Behavior 😯
Running a test containing such a request produces this error:
This is what appears to be the relevant part of the stack trace:
Steps to Reproduce 🕹
Link: https://codesandbox.io/s/create-react-app-jl67r
TextFieldthat do not failcreateNodeMockto return atextareaelementContext 🔦
I have test suites which render React components that have the Material UI
TextFieldcomponent withmultilineset to true (but norowsattribute). These tests passed in v1.x (and the app works fine as well) but are now throwing this error in v4.1.3 (I skipped v3.x so I don't know if they would have passed then).I can use the
createNodeMockworkaround for now, but it feels unwieldy to add this to every usage ofReactTestRendererfor a component that happens to have aTextField multilinesomewhere in its tree.Your Environment 🌎
Thanks in advance for any help that you might be able to provide!
The text was updated successfully, but these errors were encountered: