Skip to content

Fix ignored setState in Safari when iframe is touched - #24459

Merged
gaearon merged 1 commit into
react:mainfrom
gaearon:safari-flush-early
May 12, 2022
Merged

Fix ignored setState in Safari when iframe is touched#24459
gaearon merged 1 commit into
react:mainfrom
gaearon:safari-flush-early

Conversation

@gaearon

@gaearongaearon commented Apr 28, 2022

Copy link
Copy Markdown
Collaborator

Fixes#24458.

Just like #22459, this is caused by a Safari bug (https://bugs.webkit.org/show_bug.cgi?id=235322).

Our original solution for #22459 was #23111: if the microtask callback runs prematurely, we ignore it. This relied on the assumption that we always check for more work at the end of the task anyway. However, we don't check for more work if we aren't either rendering or committing — notably, we don't check for more work at the end of the event handler.

This fix is to allow the update to flush early. This doesn't respect batching but matches what Safari is trying to do (which is to flush everything early).

@facebook-github-botfacebook-github-bot added the React Core Team Opened by a member of the React Core Team label Apr 28, 2022
@sizebot

Copy link
Copy Markdown

Comparing: 340060c...b7b40cd

Critical size changes

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

Name+/-BaseCurrent+/- gzipBase gzipCurrent gzip
oss-stable/react-dom/cjs/react-dom.production.min.js=131.58 kB131.58 kB=42.11 kB42.11 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js=136.82 kB136.82 kB=43.69 kB43.69 kB
facebook-www/ReactDOM-prod.classic.js=441.17 kB441.18 kB=80.49 kB80.49 kB
facebook-www/ReactDOM-prod.modern.js=426.38 kB426.39 kB=78.29 kB78.29 kB
facebook-www/ReactDOMForked-prod.classic.js=441.17 kB441.18 kB=80.49 kB80.49 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against b7b40cd

queue.forEach(cb => cb());
queue = [];
flushMicrotasksPrematurely = function() {
while (queue.length > 0) {

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

This simulates the Safari behavior of trying to drain the queue until it's empty. So that we don't "fix" this by adding an infinite loop.

Proof:

<body><script>letbusy=trueconstcallback=()=>{if(busy){console.error('noo im busy')queueMicrotask(callback)}else{console.log('im ok')}}queueMicrotask(callback);console.log("will add iframe");constiframe=document.createElement("iframe");iframe.src="localhost";document.body.appendChild(iframe);console.log("did add iframe");busy=false</script></body>

gets Safari infinitely stuck

@gaearon

Copy link
Copy Markdown
CollaboratorAuthor

ping @acdlite

@gaearon
gaearon merged commit 2c8a145 into react:mainMay 12, 2022
@gaearon
gaearon deleted the safari-flush-early branch May 12, 2022 16:58
lunaruan pushed a commit to lunaruan/react that referenced this pull request May 13, 2022
facebook-github-bot pushed a commit to react/react-native that referenced this pull request Jun 6, 2022
Summary:
This sync includes the following changes:
- **[dd4950c](react/react@dd4950c90 )**: [Flight] Implement useId hook ([#24172](react/react#24172)) //<Josh Story>//
- **[26a5b3c](react/react@26a5b3c7f )**: Explicitly set `highWaterMark` to 0 for `ReadableStream` ([#24641](react/react#24641)) //<Josh Larson>//
- **[aec5759](react/react@aec575914 )**: [Fizz] Send errors down to client ([#24551](react/react#24551)) //<Josh Story>//
- **[a276638](react/react@a2766387e )**: [Fizz] Improve text separator byte efficiency ([#24630](react/react#24630)) //<Josh Story>//
- **[f786053](react/react@f7860538a )**: Fix typo in useSyncExternalStore main entry point error ([#24631](react/react#24631)) //<François Chalifour>//
- **[1bed207](react/react@1bed20731 )**: Add a module map option to the Webpack Flight Client ([#24629](react/react#24629)) //<Sebastian Markbåge>//
- **[b2763d3](react/react@b2763d3ea )**: Move hydration code out of normal Suspense path ([#24532](react/react#24532)) //<Andrew Clark>//
- **[357a613](react/react@357a61324 )**: [DevTools][Transition Tracing] Added support for Suspense Boundaries ([#23365](react/react#23365)) //<Luna Ruan>//
- **[2c8a145](react/react@2c8a1452b )**: Fix ignored setState in Safari when iframe is touched ([#24459](react/react#24459)) //<dan>//
- **[6266263](react/react@62662633d )**: Remove enableFlipOffscreenUnhideOrder ([#24545](react/react#24545)) //<Ricky>//
- **[34da5aa](react/react@34da5aa69 )**: Only treat updates to lazy as a new mount in legacy mode ([#24530](react/react#24530)) //<Ricky>//
- **[46a6d77](react/react@46a6d77e3 )**: Unify JSResourceReference Interfaces ([#24507](react/react#24507)) //<Timothy Yung>//
- **[6cbf0f7](react/react@6cbf0f7fa )**: Fork ReactSymbols ([#24484](react/react#24484)) //<Ricky>//
- **[a10a9a6](react/react@a10a9a6b5 )**: Add test for hiding children after layout destroy ([#24483](react/react#24483)) //<Ricky>//
- **[b4eb0ad](react/react@b4eb0ad71 )**: Do not replay erroring beginWork with invokeGuardedCallback when suspended or previously errored ([#24480](react/react#24480)) //<Josh Story>//
- **[99eef9e](react/react@99eef9e2d )**: Hide children of Offscreen after destroy effects ([#24446](react/react#24446)) //<Ricky>//
- **[ce13860](react/react@ce1386028 )**: Remove enablePersistentOffscreenHostContainer flag ([#24460](react/react#24460)) //<Andrew Clark>//
- **[72b7462](react/react@72b7462fe )**: Bump local package.json versions for 18.1 release ([#24447](react/react#24447)) //<Andrew Clark>//
- **[22edb9f](react/react@22edb9f77 )**: React `version` field should match package.json ([#24445](react/react#24445)) //<Andrew Clark>//
- **[6bf3dee](react/react@6bf3deef5 )**: Upgrade react-shallow-renderer to support react 18 ([#24442](react/react#24442)) //<Michael サイトー 中村 Bashurov>//
Changelog:
[General][Changed] - React Native sync for revisions bd4784c...d300ceb
jest_e2e[run_all_tests]
Reviewed By: cortinico, kacieb
Differential Revision: D36874368
fbshipit-source-id: c0ee015f4ef2fa56e57f7a1f6bc37dd05c949877
mrizwanashiq pushed a commit to mrizwanashiq/react that referenced this pull request Jun 25, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedReact Core TeamOpened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: setState is not flushed if an iframe is added in the same tick in Safari

4 participants

@gaearon@sizebot@acdlite@facebook-github-bot