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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context Menu Flashes Briefly After Hide (React 18 Batched Updates Issue) #219

Closed
sotarules opened this issue Oct 15, 2022 · 3 comments
Closed

Comments

@sotarules
Copy link

sotarules commented Oct 15, 2022

Hi! 馃憢

Firstly, thanks for your work on this project! 馃檪

Today I used patch-package to patch react-contexify@5.0.0 for the project I'm working on.

Here is the diff that solved my problem:

diff --git a/node_modules/react-contexify/dist/react-contexify.esm.js b/node_modules/react-contexify/dist/react-contexify.esm.js
index 5a371f7..90b5321 100644
--- a/node_modules/react-contexify/dist/react-contexify.esm.js
+++ b/node_modules/react-contexify/dist/react-contexify.esm.js
@@ -1,4 +1,5 @@
 import React, { useContext, createContext, useRef, useEffect, Children, cloneElement, useReducer, useState } from 'react';
+import ReactDOM from 'react-dom';
 import cx from 'clsx';
 
 function _extends() {
@@ -520,12 +521,14 @@ var Menu = function Menu(_ref) {
   }
 
   function handleAnimationEnd() {
-    if (state.willLeave && state.visible) {
-      setState({
-        visible: false,
-        willLeave: false
-      });
-    }
+    ReactDOM.flushSync(() => {
+      if (state.willLeave && state.visible) {
+        setState({
+          visible: false,
+          willLeave: false
+        });
+      }
+    })
   }
 
   function computeAnimationClasses() {

This issue body was partially generated by patch-package.

@supriome
Copy link

Thanks. @sotarules And I have a question, do we need to change this file react-contexify.cjs.production.min.js to use react-contexify in produciton.

@sotarules
Copy link
Author

sotarules commented Oct 24, 2022

@supriome no sorry you only have to patch the ESM. I am going to alter that comment to get rid of the CJS patch, that was just me experimenting. I could not figure out which file I had to change.

BTW I'm upset with React development team, specifically React 18 batch updating by default, requiring flushSync to opt out. I've been chasing problems caused by this for weeks now.

@yuri2peter
Copy link

@sotarules Thanks, that patch fixed my problem.

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

No branches or pull requests

4 participants