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

Don't fire the render phase update warning for class lifecycles #18330

Merged
merged 3 commits into from
Mar 18, 2020

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Mar 17, 2020

Fixes #18178 (comment).

In 16.13, we added a new warning: Cannot update a component from inside a body of a function component. But as we see from reports like #18178 (comment), it fired for updates from inside of class lifecycles too. While arguably these are real issues (they happen in render phase) it's pretty noisy for legacy code.

In this PR, I do the following:

  • Change the wording (from inside a body of a function component -> while rendering). This more accurately describes when it would fire.
  • Change the deduping originally added in improve error message for cross-functional component updates #18316 to be more aggressive. In particular, I suggest to not fire more than one warning per callsite. This is because the actual target components tend to be disconnected and mostly unrelated due to abstractions like Redux. Let's focus on the part that fires the updates.
  • Limit the warning to function component bodies and class render lifecycles. Everything else will get muted. This is consistent with how our existing class "setState from another component's render" warning works.
    • As a possible follow-up, we could make both of these fire for other render-phase lifecycles like getDerivedStateFromProps, shouldComponentUpdate, and constructor. Maybe in strict mode only. However, we should keep them muted for componentWill* lifecycles including render phase ones.

Here is a table of the changes:

Screenshot 2020-03-17 at 21 10 07

Red means new behavior, blue means revert to 16.12 behavior.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 17, 2020
@gaearon gaearon requested a review from acdlite March 17, 2020 21:09
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 933157f:

Sandbox Source
peaceful-mestorf-4ufj8 Configuration

@sizebot
Copy link

sizebot commented Mar 17, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 933157f

@sizebot
Copy link

sizebot commented Mar 17, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 933157f

@gaearon gaearon requested a review from bvaughn March 17, 2020 21:54
@gaearon gaearon merged commit fe1f79b into facebook:master Mar 18, 2020
@gaearon gaearon deleted the warn-less branch March 18, 2020 00:07
acdlite pushed a commit to acdlite/react that referenced this pull request Mar 19, 2020
…book#18330)

* Change the warning to not say "function body"

This warning is more generic and may happen with class components too.

* Dedupe by the rendering component

* Don't warn outside of render
acdlite pushed a commit to acdlite/react that referenced this pull request Mar 19, 2020
…book#18330)

* Change the warning to not say "function body"

This warning is more generic and may happen with class components too.

* Dedupe by the rendering component

* Don't warn outside of render
acdlite pushed a commit to acdlite/react that referenced this pull request Mar 19, 2020
…book#18330)

* Change the warning to not say "function body"

This warning is more generic and may happen with class components too.

* Dedupe by the rendering component

* Don't warn outside of render
@@ -1372,6 +1373,7 @@ function mountIndeterminateComponent(
context,
renderExpirationTime,
);
setIsRendering(false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the component throws, setIsRendering won't be reset. This needs to be reset in the the work loop. Here:

// Reset module-level state that was set during the render phase.
resetContextDependencies();
resetHooksAfterThrow();
resetCurrentDebugFiberInDEV();

Copy link
Collaborator

@acdlite acdlite Mar 19, 2020

Choose a reason for hiding this comment

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

Never mind, gets reset by resetCurrentDebugFiberInDEV

newkdr added a commit to newkdr/vscode-gitlens that referenced this pull request Dec 12, 2024
![snyk-top-banner](https://redirect.github.com/andygongea/OWASP-Benchmark/assets/818805/c518c423-16fe-447e-b67f-ad5a49b5d123)


<h3>Snyk has created this PR to upgrade react-dom from 16.8.4 to
16.14.0.</h3>

:information_source: Keep your dependencies up-to-date. This makes it
easier to fix existing vulnerabilities and to more quickly identify and
fix newly disclosed vulnerabilities when they affect your project.

<hr/>


- The recommended version is **13 versions** ahead of your current
version.

- The recommended version was released on **4 years ago**.



<details>
<summary><b>Release notes</b></summary>
<br/>
  <details>
    <summary>Package name: <b>react-dom</b></summary>
    <ul>
      <li>
<b>16.14.0</b> - <a
href="https://redirect.github.com/facebook/react/releases/tag/v16.14.0">2020-10-14</a></br><h3>React</h3>
<ul>
<li>Add support for the <a
href="https://reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html"
rel="nofollow">new JSX transform</a>. (<a
href="https://redirect.github.com/lunaruan">@ lunaruan</a> in <a
href="https://redirect.github.com/facebook/react/pull/18299"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/18299/hovercard">#18299</a>)</li>
</ul>
      </li>
      <li>
<b>16.13.1</b> - <a
href="https://redirect.github.com/facebook/react/releases/tag/v16.13.1">2020-03-19</a></br><h3>React
DOM</h3>
<ul>
<li>Fix bug in legacy mode Suspense where effect clean-up functions are
not fired. This only affects users who use Suspense for data fetching in
legacy mode, which is not technically supported. (<a
href="https://redirect.github.com/acdlite">@ acdlite</a> in <a
href="https://redirect.github.com/facebook/react/pull/18238"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/18238/hovercard">#18238</a>)</li>
<li>Revert warning for cross-component updates that happen inside class
render lifecycles (<code>componentWillReceiveProps</code>,
<code>shouldComponentUpdate</code>, and so on). (<a
href="https://redirect.github.com/gaearon">@ gaearon</a> in <a
href="https://redirect.github.com/facebook/react/pull/18330"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/18330/hovercard">#18330</a>)</li>
</ul>
<h2>Artifacts</h2>
<ul>
<li>react: <a href="https://unpkg.com/react@16.13.1/umd/"
rel="nofollow">https://unpkg.com/react@16.13.1/umd/</a></li>
<li>react-art: <a href="https://unpkg.com/react-art@16.13.1/umd/"
rel="nofollow">https://unpkg.com/react-art@16.13.1/umd/</a></li>
<li>react-dom: <a href="https://unpkg.com/react-dom@16.13.1/umd/"
rel="nofollow">https://unpkg.com/react-dom@16.13.1/umd/</a></li>
<li>react-is: <a href="https://unpkg.com/react-is@16.13.1/umd/"
rel="nofollow">https://unpkg.com/react-is@16.13.1/umd/</a></li>
<li>react-test-renderer: <a
href="https://unpkg.com/react-test-renderer@16.13.1/umd/"
rel="nofollow">https://unpkg.com/react-test-renderer@16.13.1/umd/</a></li>
<li>scheduler: <a href="https://unpkg.com/scheduler@0.19.1/umd/"
rel="nofollow">https://unpkg.com/scheduler@0.19.1/umd/</a></li>
</ul>
      </li>
      <li>
<b>16.13.0</b> - <a
href="https://redirect.github.com/facebook/react/releases/tag/v16.13.0">2020-02-26</a></br><h3>React</h3>
<ul>
<li>Warn when a string ref is used in a manner that's not amenable to a
future codemod (<a href="https://redirect.github.com/lunaruan">@
lunaruan</a> in <a
href="https://redirect.github.com/facebook/react/pull/17864"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17864/hovercard">#17864</a>)</li>
<li>Deprecate <code>React.createFactory()</code> (<a
href="https://redirect.github.com/trueadm">@ trueadm</a> in <a
href="https://redirect.github.com/facebook/react/pull/17878"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17878/hovercard">#17878</a>)</li>
</ul>
<h3>React DOM</h3>
<ul>
<li>Warn when changes in <code>style</code> may cause an unexpected
collision (<a href="https://redirect.github.com/sophiebits">@
sophiebits</a> in <a
href="https://redirect.github.com/facebook/react/pull/14181"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/14181/hovercard">#14181</a>, <a
href="https://redirect.github.com/facebook/react/pull/18002"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/18002/hovercard">#18002</a>)</li>
<li>Warn when a function component is updated during another component's
render phase (<a class="user-mention notranslate"
data-hovercard-type="user" data-hovercard-url="/users/acdlite/hovercard"
data-octo-click="hovercard-link-click"
data-octo-dimensions="link_type:self"
href="https://redirect.github.com/acdlite">@ acdlite</a> in <a
href="https://redirect.github.com/facebook/react/pull/17099"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17099/hovercard">#17099</a>)</li>
<li>Deprecate <code>unstable_createPortal</code> (<a
href="https://redirect.github.com/trueadm">@ trueadm</a> in <a
href="https://redirect.github.com/facebook/react/pull/17880"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17880/hovercard">#17880</a>)</li>
<li>Fix <code>onMouseEnter</code> being fired on disabled buttons (<a
href="https://redirect.github.com/AlfredoGJ">@ AlfredoGJ</a> in <a
href="https://redirect.github.com/facebook/react/pull/17675"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17675/hovercard">#17675</a>)</li>
<li>Call <code>shouldComponentUpdate</code> twice when developing in
<code>StrictMode</code> (<a href="https://redirect.github.com/bvaughn">@
bvaughn</a> in <a
href="https://redirect.github.com/facebook/react/pull/17942"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17942/hovercard">#17942</a>)</li>
<li>Add <code>version</code> property to ReactDOM (<a
href="https://redirect.github.com/ealush">@ ealush</a> in <a
href="https://redirect.github.com/facebook/react/pull/15780"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/15780/hovercard">#15780</a>)</li>
<li>Don't call <code>toString()</code> of
<code>dangerouslySetInnerHTML</code> (<a
href="https://redirect.github.com/sebmarkbage">@ sebmarkbage</a> in <a
href="https://redirect.github.com/facebook/react/pull/17773"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17773/hovercard">#17773</a>)</li>
<li>Show component stacks in more warnings (<a
href="https://redirect.github.com/gaearon">@ gaearon</a> in <a
href="https://redirect.github.com/facebook/react/pull/17922"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17922/hovercard">#17922</a>, <a
href="https://redirect.github.com/facebook/react/pull/17586"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17586/hovercard">#17586</a>)</li>
</ul>
<h3>Concurrent Mode (Experimental)</h3>
<ul>
<li>Warn for problematic usages of <code>ReactDOM.createRoot()</code>
(<a href="https://redirect.github.com/trueadm">@ trueadm</a> in <a
href="https://redirect.github.com/facebook/react/pull/17937"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17937/hovercard">#17937</a>)</li>
<li>Remove <code>ReactDOM.createRoot()</code> callback params and added
warnings on usage (<a href="https://redirect.github.com/bvaughn">@
bvaughn</a> in <a
href="https://redirect.github.com/facebook/react/pull/17916"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17916/hovercard">#17916</a>)</li>
<li>Don't group Idle/Offscreen work with other work (<a
href="https://redirect.github.com/sebmarkbage">@ sebmarkbage</a> in <a
href="https://redirect.github.com/facebook/react/pull/17456"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17456/hovercard">#17456</a>)</li>
<li>Adjust <code>SuspenseList</code> CPU bound heuristic (<a
href="https://redirect.github.com/sebmarkbage">@ sebmarkbage</a> in <a
href="https://redirect.github.com/facebook/react/pull/17455"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17455/hovercard">#17455</a>)</li>
<li>Add missing event plugin priorities (<a
href="https://redirect.github.com/trueadm">@ trueadm</a> in <a
href="https://redirect.github.com/facebook/react/pull/17914"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17914/hovercard">#17914</a>)</li>
<li>Fix <code>isPending</code> only being true when transitioning from
inside an input event (<a href="https://redirect.github.com/acdlite">@
acdlite</a> in <a
href="https://redirect.github.com/facebook/react/pull/17382"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17382/hovercard">#17382</a>)</li>
<li>Fix <code>React.memo</code> components dropping updates when
interrupted by a higher priority update (<a class="user-mention
notranslate" data-hovercard-type="user"
data-hovercard-url="/users/acdlite/hovercard"
data-octo-click="hovercard-link-click"
data-octo-dimensions="link_type:self"
href="https://redirect.github.com/acdlite">@ acdlite</a> in <a
href="https://redirect.github.com/facebook/react/pull/18091"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/18091/hovercard">#18091</a>)</li>
<li>Don't warn when suspending at the wrong priority (<a
href="https://redirect.github.com/gaearon">@ gaearon</a> in <a
href="https://redirect.github.com/facebook/react/pull/17971"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17971/hovercard">#17971</a>)</li>
<li>Fix a bug with rebasing updates (<a
href="https://redirect.github.com/acdlite">@ acdlite</a> and <a
href="https://redirect.github.com/sebmarkbage">@ sebmarkbage</a> in <a
href="https://redirect.github.com/facebook/react/pull/17560"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17560/hovercard">#17560</a>, <a
href="https://redirect.github.com/facebook/react/pull/17510"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17510/hovercard">#17510</a>, <a
href="https://redirect.github.com/facebook/react/pull/17483"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17483/hovercard">#17483</a>, <a
href="https://redirect.github.com/facebook/react/pull/17480"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17480/hovercard">#17480</a>)</li>
</ul>
<h2>Artifacts</h2>
<ul>
<li>react: <a href="https://unpkg.com/react@16.13.0/umd/"
rel="nofollow">https://unpkg.com/react@16.13.0/umd/</a></li>
<li>react-art: <a href="https://unpkg.com/react-art@16.13.0/umd/"
rel="nofollow">https://unpkg.com/react-art@16.13.0/umd/</a></li>
<li>react-dom: <a href="https://unpkg.com/react-dom@16.13.0/umd/"
rel="nofollow">https://unpkg.com/react-dom@16.13.0/umd/</a></li>
<li>react-is: <a href="https://unpkg.com/react-is@16.13.0/umd/"
rel="nofollow">https://unpkg.com/react-is@16.13.0/umd/</a></li>
<li>react-test-renderer: <a
href="https://unpkg.com/react-test-renderer@16.13.0/umd/"
rel="nofollow">https://unpkg.com/react-test-renderer@16.13.0/umd/</a></li>
<li>scheduler: <a href="https://unpkg.com/scheduler@0.19.0/umd/"
rel="nofollow">https://unpkg.com/scheduler@0.19.0/umd/</a></li>
</ul>
      </li>
      <li>
<b>16.12.0</b> - <a
href="https://redirect.github.com/facebook/react/releases/tag/v16.12.0">2019-11-14</a></br><h3>React
DOM</h3>
<ul>
<li>Fix passive effects (<code>useEffect</code>) not being fired in a
multi-root app. (<a href="https://redirect.github.com/acdlite">@
acdlite</a> in <a
href="https://redirect.github.com/facebook/react/pull/17347"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17347/hovercard">#17347</a>)</li>
</ul>
<h3>React Is</h3>
<ul>
<li>Fix <code>lazy</code> and <code>memo</code> types considered
elements instead of components (<a
href="https://redirect.github.com/bvaughn">@ bvaughn</a> in <a
href="https://redirect.github.com/facebook/react/pull/17278"
data-hovercard-type="pull_request"
data-hovercard-url="/facebook/react/pull/17278/hovercard">#17278</a>)</li>
</ul>
<h2>Artifacts</h2>
<p>• react: <a href="https://unpkg.com/react@16.12.0/umd/"
rel="nofollow">https://unpkg.com/react@16.12.0/umd/</a><br>
• react-art: <a href="https://unpkg.com/react-art@16.12.0/umd/"
rel="nofollow">https://unpkg.com/react-art@16.12.0/umd/</a><br>
• react-dom: <a href="https://unpkg.com/react-dom@16.12.0/umd/"
rel="nofollow">https://unpkg.com/react-dom@16.12.0/umd/</a><br>
• react-is: <a href="https://unpkg.com/react-is@16.12.0/umd/"
rel="nofollow">https://unpkg.com/react-is@16.12.0/umd/</a><br>
• react-test-renderer: <a
href="https://unpkg.com/react-test-renderer@16.12.0/umd/"
rel="nofollow">https://unpkg.com/react-test-renderer@16.12.0/umd/</a><br>
• scheduler: <a href="https://unpkg.com/scheduler@0.18.0/umd/"
rel="nofollow">https://unpkg.com/scheduler@0.18.0/umd/</a></p>
      </li>
      <li>
        <b>16.11.0</b> - 2019-10-22
      </li>
      <li>
        <b>16.10.2</b> - 2019-10-03
      </li>
      <li>
        <b>16.10.1</b> - 2019-09-28
      </li>
      <li>
        <b>16.10.0</b> - 2019-09-27
      </li>
      <li>
        <b>16.9.0</b> - 2019-08-08
      </li>
      <li>
        <b>16.9.0-rc.0</b> - 2019-08-05
      </li>
      <li>
        <b>16.9.0-alpha.0</b> - 2019-04-03
      </li>
      <li>
        <b>16.8.6</b> - 2019-03-28
      </li>
      <li>
        <b>16.8.5</b> - 2019-03-22
      </li>
      <li>
        <b>16.8.4</b> - 2019-03-05
      </li>
    </ul>
from <a
href="https://redirect.github.com/facebook/react/releases">react-dom
GitHub release notes</a>
  </details>
</details>

---

> [!IMPORTANT]
>
> - Check the changes in this PR to ensure they won't cause issues with
your project.
> - This PR was automatically created by Snyk using the credentials of a
real user.

---

**Note:** _You are seeing this because you or someone else with access
to this repository has authorized Snyk to open upgrade PRs._

**For more information:** <img
src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiI4NTdmYmVkYy1hZTE3LTQ3ZGYtODhkYy03MTcyMWYxNThjZTkiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6Ijg1N2ZiZWRjLWFlMTctNDdkZi04OGRjLTcxNzIxZjE1OGNlOSJ9fQ=="
width="0" height="0"/>

> - 🧐 [View latest project
report](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 📜 [Customise PR
templates](https://docs.snyk.io/scan-using-snyk/pull-requests/snyk-fix-pull-or-merge-requests/customize-pr-templates)
> - 🛠 [Adjust upgrade PR
settings](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr)
> - 🔕 [Ignore this dependency or unsubscribe from future upgrade
PRs](https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17/settings/integration?pkg&#x3D;react-dom&amp;utm_source&#x3D;github&amp;utm_medium&#x3D;referral&amp;page&#x3D;upgrade-pr#auto-dep-upgrades)

[//]: #
'snyk:metadata:{"customTemplate":{"variablesUsed":[],"fieldsUsed":[]},"dependencies":[{"name":"react-dom","from":"16.8.4","to":"16.14.0"}],"env":"prod","hasFixes":false,"isBreakingChange":false,"isMajorUpgrade":false,"issuesToFix":[],"prId":"857fbedc-ae17-47df-88dc-71721f158ce9","prPublicId":"857fbedc-ae17-47df-88dc-71721f158ce9","packageManager":"npm","priorityScoreList":[],"projectPublicId":"12a8a5f5-3e19-438c-8280-eb8f4ee06d17","projectUrl":"https://app.snyk.io/org/newkdr/project/12a8a5f5-3e19-438c-8280-eb8f4ee06d17?utm_source=github&utm_medium=referral&page=upgrade-pr","prType":"upgrade","templateFieldSources":{"branchName":"default","commitMessage":"default","description":"default","title":"default"},"templateVariants":[],"type":"auto","upgrade":[],"upgradeInfo":{"versionsDiff":13,"publishedDate":"2020-10-14T19:38:26.085Z"},"vulns":[]}'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: too hard to fix "Cannot update a component from inside the function body of a different component."
5 participants