-
Notifications
You must be signed in to change notification settings - Fork 46.5k
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
fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful #27365
fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful #27365
Conversation
This is the exact type of implementation detail leakage that bothers me so much about the current DevTools architecture. I'd rather DevTools check a version string and fork based on that. |
Agree, but aren't there any other ways to understand if hook can schedule an update or if it can contain state, from reconciler perspective? |
ae957ad
to
038617d
Compare
Switched to checking just |
…etermining if hook is stateful
038617d
to
2e28c42
Compare
* refactor[devtools/extension]: refactored messaging logic across different parts of the extension ([hoxyq](https://github.com/hoxyq) in [#27417](#27417)) * fix[devtools/extension]: added a workaround for proxy content script injection in firefox ([hoxyq](https://github.com/hoxyq) in [#27375](#27375)) * fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful ([hoxyq](https://github.com/hoxyq) in [#27365](#27365)) * feat[devtools/extension]: show disclaimer when page doesnt run react and refactor react polling logic ([hoxyq](https://github.com/hoxyq) in [#27373](#27373)) * feat:-Added a delete all filters action and added title to the add filter a… ([Biki-das](https://github.com/Biki-das) in [#27332](#27332)) * fix[devtools/extension]: unregister dynamically injected content scripts instead of filtering ([hoxyq](https://github.com/hoxyq) in [#27369](#27369)) * refactor[devtools/extension]: more stable element updates polling to avoid timed out errors ([hoxyq](https://github.com/hoxyq) in [#27357](#27357)) * feat[devtools/extension]: add dark theme for popup ([rakleed](https://github.com/rakleed) in [#27330](#27330))
### React upstream changes - facebook/react#27417 - facebook/react#27408 - facebook/react#27409 - facebook/react#27405 - facebook/react#27375 - facebook/react#27407 - facebook/react#27365 - facebook/react#27399 - facebook/react#27395 - facebook/react#27394 - facebook/react#27397 - facebook/react#26992 - facebook/react#27388 - facebook/react#27373 - facebook/react#27332
…etermining if hook is stateful (facebook#27365) facebook#26740 introduced regression: React DevTools doesn't record updates for `useTransition` hook. I can add more details about things on DevTools side, if needed. The root cause is https://github.com/facebook/react/blob/491aec5d6113ce5bae7c10966bc38a4a8fc091a8/packages/react-reconciler/src/ReactFiberHooks.js#L2728-L2730 React DevTools expects dispatch to be present for stateful hooks that can schedule an update - https://github.com/facebook/react/blob/2eed1328478e8c923fcb4e6abf5efbd9e1233402/packages/react-devtools-shared/src/backend/renderer.js#L1422-L1428 With these changes, we still call dispatch in `startTransition`, but also patch `queue` object with it, so that React DevTools can recognise `useTransition` as stateful hook that can schedule update. I am not sure if this is the right approach to fix this, can we distinguish if `startTransition` was called from `useTransition` hook or as a standalone function?
* refactor[devtools/extension]: refactored messaging logic across different parts of the extension ([hoxyq](https://github.com/hoxyq) in [facebook#27417](facebook#27417)) * fix[devtools/extension]: added a workaround for proxy content script injection in firefox ([hoxyq](https://github.com/hoxyq) in [facebook#27375](facebook#27375)) * fix[devtools/useTransition]: don't check for dispatch property when determining if hook is stateful ([hoxyq](https://github.com/hoxyq) in [facebook#27365](facebook#27365)) * feat[devtools/extension]: show disclaimer when page doesnt run react and refactor react polling logic ([hoxyq](https://github.com/hoxyq) in [facebook#27373](facebook#27373)) * feat:-Added a delete all filters action and added title to the add filter a… ([Biki-das](https://github.com/Biki-das) in [facebook#27332](facebook#27332)) * fix[devtools/extension]: unregister dynamically injected content scripts instead of filtering ([hoxyq](https://github.com/hoxyq) in [facebook#27369](facebook#27369)) * refactor[devtools/extension]: more stable element updates polling to avoid timed out errors ([hoxyq](https://github.com/hoxyq) in [facebook#27357](facebook#27357)) * feat[devtools/extension]: add dark theme for popup ([rakleed](https://github.com/rakleed) in [facebook#27330](facebook#27330))
…etermining if hook is stateful (#27365) #26740 introduced regression: React DevTools doesn't record updates for `useTransition` hook. I can add more details about things on DevTools side, if needed. The root cause is https://github.com/facebook/react/blob/491aec5d6113ce5bae7c10966bc38a4a8fc091a8/packages/react-reconciler/src/ReactFiberHooks.js#L2728-L2730 React DevTools expects dispatch to be present for stateful hooks that can schedule an update - https://github.com/facebook/react/blob/2eed1328478e8c923fcb4e6abf5efbd9e1233402/packages/react-devtools-shared/src/backend/renderer.js#L1422-L1428 With these changes, we still call dispatch in `startTransition`, but also patch `queue` object with it, so that React DevTools can recognise `useTransition` as stateful hook that can schedule update. I am not sure if this is the right approach to fix this, can we distinguish if `startTransition` was called from `useTransition` hook or as a standalone function? DiffTrain build for commit 56b1447.
#26740 introduced regression: React DevTools doesn't record updates for
useTransition
hook. I can add more details about things on DevTools side, if needed.The root cause is
react/packages/react-reconciler/src/ReactFiberHooks.js
Lines 2728 to 2730 in 491aec5
React DevTools expects dispatch to be present for stateful hooks that can schedule an update -
react/packages/react-devtools-shared/src/backend/renderer.js
Lines 1422 to 1428 in 2eed132
With these changes, we still call dispatch in
startTransition
, but also patchqueue
object with it, so that React DevTools can recogniseuseTransition
as stateful hook that can schedule update.I am not sure if this is the right approach to fix this, can we distinguish if
startTransition
was called fromuseTransition
hook or as a standalone function?