Skip to content

Check for infinite update loops even if unmounted #24697

New issue

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

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

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Jun 9, 2022

The infinite update loop check doesn't need to run if the component already unmounted, because an update to an unmounted component can't cause a re-render. But because we used to run the check in this case, anyway, I found one test in www that happens to "rely on" this behavior (accidentally). The test is a pretty messy snapshot thing that I have no interest fixing so to unblock the sync I'm just going to switch this back to how it was.

acdlite added 2 commits June 8, 2022 19:24
The infinite update loop check doesn't need to run if the component
already unmounted, because an update to an unmounted component can't
cause a re-render. But because we used to run the check in this case,
anyway, I found one test in www that happens to "rely on" this behavior
(accidentally). The test is a pretty messy snapshot thing that I have no
interest fixing so to unblock the sync I'm just going to switch this
back to how it was.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 9, 2022
@acdlite acdlite merged commit 8186b19 into facebook:main Jun 9, 2022
@sizebot
Copy link

sizebot commented Jun 9, 2022

Comparing: 3e92eb0...d6feb23

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 131.76 kB 131.76 kB = 42.30 kB 42.30 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 137.03 kB 137.03 kB +0.02% 43.89 kB 43.90 kB
facebook-www/ReactDOM-prod.classic.js = 441.08 kB 441.08 kB +0.02% 80.67 kB 80.68 kB
facebook-www/ReactDOM-prod.modern.js = 426.39 kB 426.39 kB +0.02% 78.47 kB 78.49 kB
facebook-www/ReactDOMForked-prod.classic.js = 441.79 kB 441.79 kB +0.02% 80.89 kB 80.90 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against d6feb23

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Jun 15, 2022
Summary:
This sync includes the following changes:
- **[5cc2487e0](facebook/react@5cc2487e0 )**: bump versions for next release ([#24725](facebook/react#24725)) //<Josh Story>//
- **[54f17e490](facebook/react@54f17e490 )**: [Transition Tracing] Fix Cache and Transitions Pop Order ([#24719](facebook/react#24719)) //<Luna Ruan>//
- **[7cf8dfd94](facebook/react@7cf8dfd94 )**: [Transition Tracing] Create/Process Marker Complete Callback ([#24700](facebook/react#24700)) //<Luna Ruan>//
- **[327e4a1f9](facebook/react@327e4a1f9 )**: [Follow-up] Land enableClientRenderFallbackOnTextMismatch //<Andrew Clark>//
- **[a8c9cb18b](facebook/react@a8c9cb18b )**: Land enableSuspenseLayoutEffectSemantics flag ([#24713](facebook/react#24713)) //<Andrew Clark>//
- **[a8555c308](facebook/react@a8555c308 )**: [Transition Tracing] Add Tracing Marker Stack ([#24661](facebook/react#24661)) //<Luna Ruan>//
- **[8186b1937](facebook/react@8186b1937 )**: Check for infinite update loops even if unmounted ([#24697](facebook/react#24697)) //<Andrew Clark>//
- **[060505e9d](facebook/react@060505e9d )**: Fix misapplying prod error opt-out ([#24688](facebook/react#24688)) //<Josh Story>//
- **[47944142f](facebook/react@47944142f )**: `now` isn't part of the react-reconciler config anymore ([#24689](facebook/react#24689)) //<Mathieu Dutour>//
- **[b34552352](facebook/react@b34552352 )**: [Fizz] Support abort reasons ([#24680](facebook/react#24680)) //<Josh Story>//
- **[79f54c16d](facebook/react@79f54c16d )**: Bugfix: Revealing a hidden update ([#24685](facebook/react#24685)) //<Andrew Clark>//
- **[7e8a020a4](facebook/react@7e8a020a4 )**: Remove extra Server Context argument ([#24683](facebook/react#24683)) //<Sebastian Markbåge>//
- **[4f29ba1cc](facebook/react@4f29ba1cc )**: support errorInfo in onRecoverableError ([#24591](facebook/react#24591)) //<Josh Story>//
- **[1cd90d2cc](facebook/react@1cd90d2cc )**: Refactor of interleaved ("concurrent") update queue ([#24663](facebook/react#24663)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions d300ceb...256aefb

jest_e2e[run_all_tests]

Reviewed By: cortinico

Differential Revision: D37155957

fbshipit-source-id: 4c0afc95abe8fa13c3803584922c8dc0059ff562
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Aug 15, 2022
Summary:
This sync includes the following changes:
- **[a8c9cb18b](facebook/react@a8c9cb18b )**: Land enableSuspenseLayoutEffectSemantics flag ([#24713](facebook/react#24713)) //<Andrew Clark>//
- **[a8555c308](facebook/react@a8555c308 )**: [Transition Tracing] Add Tracing Marker Stack ([#24661](facebook/react#24661)) //<Luna Ruan>//
- **[8186b1937](facebook/react@8186b1937 )**: Check for infinite update loops even if unmounted ([#24697](facebook/react#24697)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 9e3b772...a8c9cb1

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D38617670

fbshipit-source-id: 59799644c24325c0c35770b174e78230c4166425
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
This sync includes the following changes:
- **[a8c9cb18b](facebook/react@a8c9cb18b )**: Land enableSuspenseLayoutEffectSemantics flag ([facebook#24713](facebook/react#24713)) //<Andrew Clark>//
- **[a8555c308](facebook/react@a8555c308 )**: [Transition Tracing] Add Tracing Marker Stack ([facebook#24661](facebook/react#24661)) //<Luna Ruan>//
- **[8186b1937](facebook/react@8186b1937 )**: Check for infinite update loops even if unmounted ([facebook#24697](facebook/react#24697)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 9e3b772...a8c9cb1

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D38617670

fbshipit-source-id: 59799644c24325c0c35770b174e78230c4166425
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
This sync includes the following changes:
- **[a8c9cb18b](facebook/react@a8c9cb18b )**: Land enableSuspenseLayoutEffectSemantics flag ([facebook#24713](facebook/react#24713)) //<Andrew Clark>//
- **[a8555c308](facebook/react@a8555c308 )**: [Transition Tracing] Add Tracing Marker Stack ([facebook#24661](facebook/react#24661)) //<Luna Ruan>//
- **[8186b1937](facebook/react@8186b1937 )**: Check for infinite update loops even if unmounted ([facebook#24697](facebook/react#24697)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions 9e3b772...a8c9cb1

jest_e2e[run_all_tests]

Reviewed By: yungsters

Differential Revision: D38617670

fbshipit-source-id: 59799644c24325c0c35770b174e78230c4166425
# for free to join this conversation on GitHub. Already have an account? # 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.

3 participants