Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call gDSFP with the right state in react-test-render #13030

Conversation

fatfisz
Copy link
Contributor

@fatfisz fatfisz commented Jun 12, 2018

Fixes #13027.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@@ -915,6 +915,39 @@ describe('ReactShallowRenderer', () => {
expect(result.props.children).toEqual(3);
});

it('should call getDerivedStateFromProps with the state set by setState', () => {
class SimpleComponent extends React.Component {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this based on any existing test in other renderers? If not, can you please find an appropriate existing test and then use it as a base?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look for that!

Copy link
Contributor Author

@fatfisz fatfisz Jun 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only test I could find that is related to this was: "should not override state with stale values if prevState is spread within getDerivedStateFromProps" in react/packages/react-dom/src/__tests__/ReactComponentLifeCycle-test.js. I modified it a bit (it involved a parent and a child component, but in the case of shallow rendering this seemed to be a bit too complicated).

@gaearon gaearon merged commit 945fc1b into facebook:master Jun 12, 2018
@fatfisz fatfisz deleted the fix-react-test-renderer-with-setState-and-gDSFP branch June 13, 2018 10:10
@fatfisz
Copy link
Contributor Author

fatfisz commented Jun 13, 2018

Just wondering, when will you be cutting a new release?

@gaearon
Copy link
Collaborator

gaearon commented Jun 13, 2018

Maybe later this week

bors bot added a commit to mozilla/delivery-console that referenced this pull request Jun 13, 2018
193: Update dependency react to v16.4.1 r=magopian a=renovate[bot]

This Pull Request updates dependency [react](https://github.com/facebook/react) from `v16.4.0` to `v16.4.1`



<details>
<summary>Release Notes</summary>

### [`v16.4.1`](https://github.com/facebook/react/blob/master/CHANGELOG.md#&#8203;1641-June-13-2018)
[Compare Source](facebook/react@v16.4.0...v16.4.1)
##### React

* You can now assign `propTypes` to components returned by `React.ForwardRef`. ([@&#8203;bvaughn] in [#&#8203;12911](`facebook/react#12911))
##### React DOM

* Fix a crash when the input `type` changes from some other types to `text`. ([@&#8203;spirosikmd] in [#&#8203;12135](`facebook/react#12135))
* Fix a crash in IE11 when restoring focus to an SVG element. ([@&#8203;ThaddeusJiang] in [#&#8203;12996](`facebook/react#12996))
* Fix a range input not updating in some cases. ([@&#8203;Illu] in [#&#8203;12939](`facebook/react#12939))
* Fix input validation triggering unnecessarily in Firefox. ([@&#8203;nhunzaker] in [#&#8203;12925](`facebook/react#12925))
* Fix an incorrect `event.target` value for the `onChange` event in IE9. ([@&#8203;nhunzaker] in [#&#8203;12976](`facebook/react#12976))
* Fix a false positive error when returning an empty `<React.Fragment />` from a component. ([@&#8203;philipp-spiess] in [#&#8203;12966](`facebook/react#12966))
##### React DOM Server

* Fix an incorrect value being provided by new context API. ([@&#8203;ericsoderberghp] in [#&#8203;12985](`facebook/react#12985), [@&#8203;gaearon] in [#&#8203;13019](`facebook/react#13019))
##### React Test Renderer

* Allow multiple root children in test renderer traversal API. ([@&#8203;gaearon] in [#&#8203;13017](`facebook/react#13017))
* Fix `getDerivedStateFromProps()` in the shallow renderer to not discard the pending state. ([@&#8203;fatfisz] in [#&#8203;13030](`facebook/react#13030))

---

</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).

Co-authored-by: Renovate Bot <bot@renovateapp.com>
bors bot added a commit to mythmon/corsica-tree-status that referenced this pull request Jun 14, 2018
25: Update react monorepo to v16.4.1 r=renovate[bot] a=renovate[bot]

This Pull Request renovates the package group "react monorepo".


-   [react-dom](https://github.com/facebook/react) (`dependencies`): from `16.4.0` to `16.4.1`
-   [react](https://github.com/facebook/react) (`dependencies`): from `16.4.0` to `16.4.1`

# Release Notes
<details>
<summary>facebook/react</summary>

### [`v16.4.1`](https://github.com/facebook/react/blob/master/CHANGELOG.md#&#8203;1641-June-13-2018)
[Compare Source](facebook/react@v16.4.0...v16.4.1)
##### React

* You can now assign `propTypes` to components returned by `React.ForwardRef`. ([@&#8203;bvaughn] in [#&#8203;12911](`facebook/react#12911))
##### React DOM

* Fix a crash when the input `type` changes from some other types to `text`. ([@&#8203;spirosikmd] in [#&#8203;12135](`facebook/react#12135))
* Fix a crash in IE11 when restoring focus to an SVG element. ([@&#8203;ThaddeusJiang] in [#&#8203;12996](`facebook/react#12996))
* Fix a range input not updating in some cases. ([@&#8203;Illu] in [#&#8203;12939](`facebook/react#12939))
* Fix input validation triggering unnecessarily in Firefox. ([@&#8203;nhunzaker] in [#&#8203;12925](`facebook/react#12925))
* Fix an incorrect `event.target` value for the `onChange` event in IE9. ([@&#8203;nhunzaker] in [#&#8203;12976](`facebook/react#12976))
* Fix a false positive error when returning an empty `<React.Fragment />` from a component. ([@&#8203;philipp-spiess] in [#&#8203;12966](`facebook/react#12966))
##### React DOM Server

* Fix an incorrect value being provided by new context API. ([@&#8203;ericsoderberghp] in [#&#8203;12985](`facebook/react#12985), [@&#8203;gaearon] in [#&#8203;13019](`facebook/react#13019))
##### React Test Renderer

* Allow multiple root children in test renderer traversal API. ([@&#8203;gaearon] in [#&#8203;13017](`facebook/react#13017))
* Fix `getDerivedStateFromProps()` in the shallow renderer to not discard the pending state. ([@&#8203;fatfisz] in [#&#8203;13030](`facebook/react#13030))

---


</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).

Co-authored-by: Renovate Bot <bot@renovateapp.com>
@kamronbatman
Copy link
Contributor

kamronbatman commented Jun 15, 2018

@gaearon, please cut a new version asap ❤️.

@gaearon
Copy link
Collaborator

gaearon commented Jun 15, 2018

It was included in 16.4.1.

@kamronbatman
Copy link
Contributor

@gaearon, I am still having a similar issue. It might be due to a different reason then (I am not sure at this time).
Thank you!

@gaearon
Copy link
Collaborator

gaearon commented Jun 16, 2018

Can you file a bug with reproducing case please?

@gaearon
Copy link
Collaborator

gaearon commented Jun 16, 2018

Or maybe you're getting confused about which behavior is intended?
There was a bug in 16.3 which you might accidentally have relied on.

See:

https://reactjs.org/blog/2018/05/23/react-v-16-4.html#bugfix-for-getderivedstatefromprops

and

https://reactjs.org/blog/2018/06/07/you-probably-dont-need-derived-state.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants