[ForwardRef in snapshot

[VincentLanglet]

When I upgrade to @material-ui/core 4 and start using @material-ui/styles, every snapshots failed.

 Received value does not match stored snapshot "[Component] Menu should render correctly 1".

    - Snapshot
    + Received

    @@ -1,6 +1,6 @@
    - <WithStyles(Paper)
    + <WithStyles(ForwardRef(Paper))
        square={false}
        style={
          Object {
            "left": 0,
            "position": "absolute",

The code didn't change at all.

const Menu = (props: MenuProps<OptionType>): ReactElement => (
  <Paper square={false} style={{ position: 'absolute', zIndex: 1, left: 0, right: 0 }} {...props.innerProps}>
    {props.children}
  </Paper>
);

Is it normal ? Can we do something about it ? I think the ForwardRef in the snapshot add useless complexity.

I even have sometime have both WithStyle and ForwardRef appearing in the snapshot since the update.

 @@ -1,6 +1,6 @@
    - <TextField
    + <WithStyles(ForwardRef(TextField))
        InputLabelProps={
          Object {
            "shrink": true,
          }
        }

[eps1lon]

That is an inherent flaw with snapshot testing. If you serialize your snapshot with implementation details (or test them in general) they can fail at any time.

I think the ForwardRef in the snapshot add useless complexity.

Depends. They do change the API of the component. They're only useless if you don't care about refs.

[VincentLanglet]

@eps1lon So If I correctly understand, you changed the implementation of all MaterialUi Component and add ForwardRef to every one, that's why I see this now in the snapshot ?

Thanks for your answer

[eps1lon]

@eps1lon So If I correctly understand, you changed the implementation of all MaterialUi Component and add ForwardRef to every one, that's why I see this now in the snapshot ?

Yes. It wasn't tagged as a breaking change in the release notes though. The PR properly marked itself as breaking though. Will fix that.

It's hard to recommend a silver bullet to snapshot testing with 3rd party libraries. Depending on your test it's probably best to serialize them shallow with the name of the identifier i.e. if you import it as MuiPaper it should be serialized with that name instead of its displayName. I don't know if there are existing solutions for this.

Serializing the whole component tree of 3rd party components is brittle. Imagine those components only introduce a headless component that is essentially an identity function i.e. const MeaningLess = props => props.children. Nothing changed about the public API (unless perf is part of your public API) but the snapshot will fail regardless.

I used snapshot testing personally but it gives you a false sense of security. It leads to throwaway test like it renders correctly that only give you a nice coverage number but no confidence in the behavior. They can be done right but just like any other test they need some setup.

[VincentLanglet]

I agree, I mainly use the snapshot to check I have the right components with the right props and the right style (I made a makeStyles mock to display all css properties passed to the component) in order to catch some UI regressions but I have others tests checking about the behavior.

[oliviertassinari]

I made a makeStyles mock to display all css properties passed to the component

@VincentLanglet We have a Jest issue focusing on improving the snapshot support: #14357. Do you think that you would be able to share the solution there?