1.4.1 Texfield multiline jest snapshot tests broken

[sakulstra]

When upgrading to 1.4.1 all our snapshot tests including a <TextField multiline /> are broken due to Error: Uncaught [TypeError: Cannot read property 'scrollHeight' of null].
We had similar issues in the past which were related to refs + react-test-renderer, but I couldn't spot any change in 1.4.1 yet which changed the ref logic.

• related Text Field fails on Jest snapshot test, with error Cannot read property 'scrollHeight' of null #5531

• This is a v1.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

minor version jump shouldn't break testing setup

Current Behavior

testing setup is broken

Steps to Reproduce

Snapshot <TextField multiline {...props} /> via jest.

Context

We render snapshots of all our pages to spot changes on component updates.

Your Environment

Tech	Version
Material-UI	v1.4.1
React	latest

[oliviertassinari]

@sakulstra I have removed some defensive checks that looks very much like dead code to me in #12238 and #12239. I doubt the root of the issue is on Material-UI. The component can be mounted just fine on a browser environment.

[kanzelm3]

I am having this issue as well. The issue is caused by the syncHeightWithShadow function inside of the Textarea component because it is attempting to access it's refs without ensuring that they are actually mounted.

https://github.com/mui-org/material-ui/blob/4eb6628e5be27a6c140c774069ea794d091c48c3/packages/material-ui/src/Input/Textarea.js#L121-L130

Our snapshot tests use Jest/Enzyme to do shallow rendering, which means child components are not mounted so the refs are null. There is currently no way for us to fix tests affected by this issue, save for just mocking the TextArea component, but that would cause the quality of the test to suffer.

[oliviertassinari]

Our snapshot tests use Jest/Enzyme to do shallow rendering, which means child components are not mounted so the refs are null.

@kanzelm3 This error comes from the componentDidMount() hook being called. Don't you find that wrong? I have faced the same issue with the Material-UI test suite. I have added: disableLifecycleMethods: true for enzyme. IMHO it should be the default value.

[oliviertassinari]

Are you OK closing the issue for enzymejs/enzyme#1411?

[kanzelm3]

@oliviertassinari I thought about disabling lifecycle methods. The problem I'm facing with that is we depend on the lifecycle methods to perform functional testing on a few of our components. Because we use storybook and the storyshots addon to perform snapshot testing, if I disable lifecycle methods, I will have to disable them for the entire project. I haven't figured out how to disable them on a per component basis yet, not sure it's possible.

[oliviertassinari]

@kanzelm3 With the information I have at hand, I deeply think it's an enzyme issue, but let's make the life easier for everybody. I think that we should be adding the defensive branch logic back 😢 (with a comment this time).