shallow renderer: wrong initial state passed to getDerivedStateFromProps

[ljharb]

Demonstrated in enzymejs/enzyme#2027 - when gDSFP returns a state change, cDU does not seem to be called by the shallow renderer. I'm not sure how to make a simple test case; but the identical enzyme tests pass for mount (ie, when using React itself) but fail for shallow (when using the shallow renderer). enzyme doesn't have any logic for calling or interacting with getDerivedStateFromProps, so I suspect it's not an issue with enzyme (but it might be!)

[miraage]

Could you please setup a sandbox?

[gaearon]

Happy to take a bugfix if you think it's a bug. (See existing shallow renderer tests)

[ljharb]

I'm actually seeing two issues here:

https://codesandbox.io/s/m7nn4v428 shows the behavior of React itself:

1. Demo's gDSFP is called, returns null
   → NOTE: called with second argument as Demo's initial this.state
2. Demo component renders (via SetProps component)
3. Demo's props are updated (by SetProps)
4. Demo's gDSFP is called, returns null
5. rerenders
6. cDU is called

https://codesandbox.io/s/mqm4o7m7nx shows the behavior of the shallow renderer:

1. Demo's gDSFP is called, returns null
   → NOTE: called with second argument as undefined
2. Demo component renders (via shallow renderer render call)
3. Demo's props are updated (by another shallow renderer render call)
4. Demo's gDSFP is called, returns null
5. rerenders
6. cDU is NOT called

(I'm not sure where to start with a PR for these fixes; if someone can help by either implementing the fix, or sharing a branch with failing tests so i can build a fix on top of that, or a direct link to where the tests and fix would go, that would be great)

[gaearon]

Here's an example of a recent gDSFP fix, it shows where to change the code and where the tests go.
https://github.com/facebook/react/pull/14613/files

[ghost]

I'll look into this.

[gaearon]

Looking at it again — this works as intended.

Shallow renderer has never fired componentDidUpdate at all, similar to how it never fired componentDidMount. The reason is that most componentDidMount and componentDidUpdate methods have side effects or deal with refs, and so they break shallow rendering assumptions anyway.

There is a bigger discussion to have about the future status of shallow renderer in and its design, but the reported issue is not a bug.

[ljharb]

@gaearon note as well the discrepancy in the first call to gDSFP - should i file a new issue, or can this be reopened?

[gaearon]

Oops, missed that. Thanks.

[gaearon]

Hmm, I can't repro with https://codesandbox.io/s/mqm4o7m7nx:

I don't see undefined.

[gaearon]

Is it only if you omit this.state assignment in constructor? This normally warns. (Although not in shallow.) I see a difference in null vs undefined for this case but I don't know how to repro {} vs undefined.

[ljharb]

Hmm - now I can't reproduce it even on my original sandboxes :-/ I see the same minor discrepancy as you when I comment out the state assignment in the Demo constructor, and while that might be nice to fix it's probably not worth the effort.

I'm going to close this - I'll comment here, or file a new issue, if I see anything else.