This repository has been archived by the owner on Sep 10, 2022. It is now read-only.
Remove useless optimisation #736
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 30, 2018
goto-bus-stop
pushed a commit
to u-wave/react-server-list
that referenced
this pull request
Aug 30, 2018
## Version **0.30.0** of **recompose** was just published.
<table>
<tr>
<th align=left>
Dependency
</th>
<td>
<a target=_blank href=https://github.com/acdlite/recompose>recompose</a>
</td>
</tr>
<tr>
<th align=left>
Current Version
</th>
<td>
0.29.0
</td>
</tr>
<tr>
<th align=left>
Type
</th>
<td>
dependency
</td>
</tr>
</table>
The version **0.30.0** is **not covered** by your **current version range**.
If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.
It might be worth looking into these changes and trying to get this project onto the latest version of recompose.
If you have a solid test suite and good coverage, a passing build is a strong indicator that you can take advantage of these changes directly by merging the proposed change into your project. If the build fails or you don’t have such unconditional trust in your tests, this branch is a great starting point for you to work on the update.
---
<details>
<summary>Release Notes</summary>
<strong>withStateHandler behaviour change</strong>
<p>Remove useless in React 16 optimisation <a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="355508627" data-permission-text="Issue title is private" data-url="acdlite/recompose#736" href="https://urls.greenkeeper.io/acdlite/recompose/pull/736#issue-212019167">#736 (comment)</a><br>
Make state change more similar to React setState.</p>
</details>
<details>
<summary>FAQ and help</summary>
There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>
---
Your [Greenkeeper](https://greenkeeper.io) bot 🌴
goto-bus-stop
pushed a commit
to u-wave/web
that referenced
this pull request
Aug 30, 2018
## Version **0.30.0** of **recompose** was just published.
<table>
<tr>
<th align=left>
Dependency
</th>
<td>
<a target=_blank href=https://github.com/acdlite/recompose>recompose</a>
</td>
</tr>
<tr>
<th align=left>
Current Version
</th>
<td>
0.29.0
</td>
</tr>
<tr>
<th align=left>
Type
</th>
<td>
dependency
</td>
</tr>
</table>
The version **0.30.0** is **not covered** by your **current version range**.
If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.
It might be worth looking into these changes and trying to get this project onto the latest version of recompose.
If you have a solid test suite and good coverage, a passing build is a strong indicator that you can take advantage of these changes directly by merging the proposed change into your project. If the build fails or you don’t have such unconditional trust in your tests, this branch is a great starting point for you to work on the update.
---
<details>
<summary>Release Notes</summary>
<strong>withStateHandler behaviour change</strong>
<p>Remove useless in React 16 optimisation <a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="355508627" data-permission-text="Issue title is private" data-url="acdlite/recompose#736" href="https://urls.greenkeeper.io/acdlite/recompose/pull/736#issue-212019167">#736 (comment)</a><br>
Make state change more similar to React setState.</p>
</details>
<details>
<summary>FAQ and help</summary>
There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>
---
Your [Greenkeeper](https://greenkeeper.io) bot 🌴
|
Hi! I started getting this error when I upgraded to version 0.30.0. I think this PR might be the culprit. Do you know how I can fix this error? |
|
Just check your code, what causes setState on render. Usually this happens because of incorrect usage of setState, inside lifecycle handlers like |
|
Hm, ok. I'm using React 16.4.0. I do use setState in Is there any harm in leaving this optimization in place given that some of us depend on its behavior? |
No without example where will be shown that exactly removal of this optimisation caused the issue and the example code correct. As componentWillReceiveProps is deprecated in React. |
|
No without example where will be shown that exactly removal of this
optimisation caused the issue
This is the only major change since the last release. What else could it
be?
You’re correct that cwrp is being deprecated but it won’t be removed until
React version 17. So until then maybe it makes sense to not break the
existing apps with this optimization? We need time to upgrade.
Removing this feature is not super critical right now. It’s not doing
anyone any harm even if it’s encouraging a bad practice. Why not wait until
React 17 and release it as part of a major version upgrade?
On Tue, Oct 23, 2018 at 1:33 PM Ivan Starkov ***@***.***> wrote:
Is there any harm in leaving this optimization in place given that some of
us depend on its behavior?
No without example where will be shown that exactly removal of this
optimisation caused the issue and the example code correct.
As componentWillReceiveProps is deprecated in React.
Also about bad practice - if you update state of the parent in cwrp
depending on props it can cause infinite loop - its overall not good (but
not so bad) practice to change state depending on props change. Here in
issues there were few examples about. Same for cdu lifecycle having that it
can cause loop changing its own state (not parent only)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#736 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAJQMOVSfK8AQtIVgHiK9PHmxv3vNYJpks5un1LUgaJpZM4WTJGD>
.
--
*Volkan Unsal*
*Product Engineer*
volkanunsal.com
|
I'm more than 20 years in development, be sure I know the answer on "What else could it be". |
|
Come on now. You can't be serious. |
|
I'll take my time to test it more carefully, and maybe you're right. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Removed optimisation is useless in React 16. The only intent for it was to not update if null was returned in handler.
But returning same value didn't caused update too, this is not same as React setState works.
This is possibly a breaking change.