fix[dynamic-scripts-injection]: unregister content scripts before registration #26765

hoxyq · 2023-05-03T09:30:32Z

Summary

DevTools is failing to inject __REACT_DEVTOOLS_GLOBAL_HOOK__ hook in incognito mode. This is not happening straight-forward, but if extension is toggled on and off, the next time I try to open it I am receiving an error that content script was already registered.

Unregistering content scripts before attempting to register them again. We need to inject __REACT_DEVTOOLS_GLOBAL_HOOK__ on each page, so this should be expected behaviour.
Fixed error logging

How did you test this change?

Local build of extension for Chrome, trying the same steps, which resulted in an error.
No regression in performance, tested on react.dev, still the same.

mondaychen

overall looks good

mondaychen · 2023-05-03T13:28:31Z

packages/react-devtools-extensions/src/background.js

+    // For some reason dynamically injected scripts might be already registered
+    // Registering them again will fail, which will result into
+    // __REACT_DEVTOOLS_GLOBAL_HOOK__ hook not being injected
+    await chrome.scripting.unregisterContentScripts();


Shall we do unregisterContentScripts({ids: contentScriptsToInject.map(s => s.id)}) to be safer?

Shall we do unregisterContentScripts({ids: contentScriptsToInject.map(s => s.id)}) to be safer?

Thought about doing this. From Chromium docs, they guarantee that only dynamically injected scripts will be unregistered.

But will add this, makes sense, just for safety

…istration

Includes the following upstream changes: - [b7972822b](https://github.com/facebook/react/commits/b7972822b) useOptimisticState -> useOptimistic ([vercel#26772](facebook/react#26772)) (Andrew Clark) - [388686f29](https://github.com/facebook/react/commits/388686f29) Add "canary" to list of allowed npm dist tags ([vercel#26767](facebook/react#26767)) (Andrew Clark) - [8a25302c6](https://github.com/facebook/react/commits/8a25302c6) fix[dynamic-scripts-injection]: unregister content scripts before registration ([vercel#26765](facebook/react#26765)) (Ruslan Lesiutin) - [2c2476834](https://github.com/facebook/react/commits/2c2476834) Rename "next" prerelease channel to "canary" ([vercel#26761](facebook/react#26761)) (Andrew Clark) - [fa4314841](https://github.com/facebook/react/commits/fa4314841) Remove deprecated workflow key from Circle config ([#26762](facebook/react#26762)) (Andrew Clark)

Includes the following upstream changes: - [b7972822b](https://github.com/facebook/react/commits/b7972822b) useOptimisticState -> useOptimistic ([#26772](facebook/react#26772)) (Andrew Clark) - [388686f29](https://github.com/facebook/react/commits/388686f29) Add "canary" to list of allowed npm dist tags ([#26767](facebook/react#26767)) (Andrew Clark) - [8a25302c6](https://github.com/facebook/react/commits/8a25302c6) fix[dynamic-scripts-injection]: unregister content scripts before registration ([#26765](facebook/react#26765)) (Ruslan Lesiutin) - [2c2476834](https://github.com/facebook/react/commits/2c2476834) Rename "next" prerelease channel to "canary" ([#26761](facebook/react#26761)) (Andrew Clark) - [fa4314841](https://github.com/facebook/react/commits/fa4314841) Remove deprecated workflow key from Circle config ([#26762](facebook/react#26762)) (Andrew Clark) - [5dd90c562](https://github.com/facebook/react/commits/5dd90c562) Use content hash for react-native builds ([#26734](facebook/react#26734)) (Samuel Susla) - [559e83aeb](https://github.com/facebook/react/commits/559e83aeb) [Fizz] Allow an action provide a custom set of props to use for progressive enhancement ([#26749](facebook/react#26749)) (Sebastian Markbåge) - [67f4fb021](https://github.com/facebook/react/commits/67f4fb021) Allow forms to skip hydration of hidden inputs ([#26735](facebook/react#26735)) (Sebastian Markbåge) - [8ea96ef84](https://github.com/facebook/react/commits/8ea96ef84) [Fizz] Encode external fizz runtime into chunks eagerly ([#26752](facebook/react#26752)) (Josh Story) - [491aec5d6](https://github.com/facebook/react/commits/491aec5d6) Implement experimental_useOptimisticState ([#26740](facebook/react#26740)) (Andrew Clark) - [9545e4810](https://github.com/facebook/react/commits/9545e4810) Add nonce support to bootstrap scripts and external runtime ([#26738](facebook/react#26738)) (Dan Ott) - [86b0e9199](https://github.com/facebook/react/commits/86b0e9199) Gate DevTools test to fix CI ([#26742](facebook/react#26742)) (Andrew Clark) - [b12bea62d](https://github.com/facebook/react/commits/b12bea62d) Preinits should support a nonce option ([#26744](facebook/react#26744)) (Josh Story) - [efbd68511](https://github.com/facebook/react/commits/efbd68511) Remove unused `initialStatus` parameter from `useHostTransitionStatus` ([#26743](facebook/react#26743)) (Sebastian Silbermann) - [18282f881](https://github.com/facebook/react/commits/18282f881) Fix: Update while suspended fails to interrupt ([#26739](facebook/react#26739)) (Andrew Clark) - [540bab085](https://github.com/facebook/react/commits/540bab085) Implement experimental_useFormStatus ([#26722](facebook/react#26722)) (Andrew Clark) ---------

List of changes: * DevTools: fix backend activation ([hoxyq](https://github.com/hoxyq) in [#26779](#26779)) * fix[dynamic-scripts-injection]: unregister content scripts before registration ([hoxyq](https://github.com/hoxyq) in [#26765](#26765))

Summary: List of changes: * DevTools: fix backend activation ([hoxyq](https://github.com/hoxyq) in [#26779](facebook/react#26779)) * fix[dynamic-scripts-injection]: unregister content scripts before registration ([hoxyq](https://github.com/hoxyq) in [#26765](facebook/react#26765)) DiffTrain build for commit facebook/react@783e7fc. Changelog: [Internal] << DO NOT EDIT BELOW THIS LINE >> Reviewed By: sammy-SC Differential Revision: D45575104 Pulled By: tyao1 fbshipit-source-id: c065eb8f1f200170f611b5becc0332109f22ef71

Summary: List of changes: * DevTools: fix backend activation ([hoxyq](https://github.com/hoxyq) in [facebook#26779](facebook/react#26779)) * fix[dynamic-scripts-injection]: unregister content scripts before registration ([hoxyq](https://github.com/hoxyq) in [facebook#26765](facebook/react#26765)) DiffTrain build for commit facebook/react@783e7fc. Changelog: [Internal] << DO NOT EDIT BELOW THIS LINE >> Reviewed By: sammy-SC Differential Revision: D45575104 Pulled By: tyao1 fbshipit-source-id: c065eb8f1f200170f611b5becc0332109f22ef71

…pts instead of filtering (#27369) Same as #26765. Related bug report - https://bugs.chromium.org/p/chromium/issues/detail?id=1393762. This was changed in #27215, when I have refactored background logic in the extension. I've missed this case, because the extension was working in incognito mode. Turns out, it stops working after the first reload, and it stops only in incognito mode, so I am rolling out back the previous workaround. Tested on Chrome that it fixes the extension in incognito mode and that it doesn't affect default mode.

…istration (facebook#26765) ## Summary Fixes facebook#26756. DevTools is failing to inject `__REACT_DEVTOOLS_GLOBAL_HOOK__` hook in incognito mode. This is not happening straight-forward, but if extension is toggled on and off, the next time I try to open it I am receiving an error that content script was already registered. <img width="676" alt="Screenshot 2023-05-02 at 14 36 53" src="https://user-images.githubusercontent.com/28902667/235877692-51c5d284-79d9-4b00-b62e-d25d5bb5e056.png"> - Unregistering content scripts before attempting to register them again. We need to inject `__REACT_DEVTOOLS_GLOBAL_HOOK__` on each page, so this should be expected behaviour. - Fixed error logging ## How did you test this change? Local build of extension for Chrome, trying the same steps, which resulted in an error. No regression in performance, tested on react.dev, still the same.

List of changes: * DevTools: fix backend activation ([hoxyq](https://github.com/hoxyq) in [facebook#26779](facebook#26779)) * fix[dynamic-scripts-injection]: unregister content scripts before registration ([hoxyq](https://github.com/hoxyq) in [facebook#26765](facebook#26765))

…pts instead of filtering (facebook#27369) Same as facebook#26765. Related bug report - https://bugs.chromium.org/p/chromium/issues/detail?id=1393762. This was changed in facebook#27215, when I have refactored background logic in the extension. I've missed this case, because the extension was working in incognito mode. Turns out, it stops working after the first reload, and it stops only in incognito mode, so I am rolling out back the previous workaround. Tested on Chrome that it fixes the extension in incognito mode and that it doesn't affect default mode.

…istration (#26765) ## Summary Fixes #26756. DevTools is failing to inject `__REACT_DEVTOOLS_GLOBAL_HOOK__` hook in incognito mode. This is not happening straight-forward, but if extension is toggled on and off, the next time I try to open it I am receiving an error that content script was already registered. <img width="676" alt="Screenshot 2023-05-02 at 14 36 53" src="https://user-images.githubusercontent.com/28902667/235877692-51c5d284-79d9-4b00-b62e-d25d5bb5e056.png"> - Unregistering content scripts before attempting to register them again. We need to inject `__REACT_DEVTOOLS_GLOBAL_HOOK__` on each page, so this should be expected behaviour. - Fixed error logging ## How did you test this change? Local build of extension for Chrome, trying the same steps, which resulted in an error. No regression in performance, tested on react.dev, still the same. DiffTrain build for commit 8a25302.

facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels May 3, 2023

hoxyq marked this pull request as ready for review May 3, 2023 10:39

mondaychen reviewed May 3, 2023

View reviewed changes

fix[dynamic-scripts-injection]: unregister content scripts before reg…

7158a38

…istration

hoxyq force-pushed the devtools/fixed-dynamic-scripts-double-injection branch from d8bd041 to 7158a38 Compare May 3, 2023 16:16

hoxyq merged commit 8a25302 into facebook:main May 3, 2023

acdlite mentioned this pull request May 3, 2023

Upgrade React to 18.3.0-canary-b7972822b-20230503 vercel/next.js#49158

Merged

rajnish307 mentioned this pull request May 4, 2023

React dev tools not working when running the application/website on incognito mode #26766

Closed

hoxyq mentioned this pull request May 4, 2023

React DevTools 4.27.6 -> 4.27.7 #26780

Merged

Azeirah mentioned this pull request May 26, 2023

[DevTools Bug]: App not recognized in Firefox, but works in Chrome #26860

Closed

hoxyq deleted the devtools/fixed-dynamic-scripts-double-injection branch June 22, 2023 10:45

hoxyq mentioned this pull request Sep 13, 2023

fix[devtools/extension]: unregister dynamically injected content scripts instead of filtering #27369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix[dynamic-scripts-injection]: unregister content scripts before registration #26765

fix[dynamic-scripts-injection]: unregister content scripts before registration #26765

hoxyq commented May 3, 2023

mondaychen left a comment

mondaychen May 3, 2023

hoxyq May 3, 2023

fix[dynamic-scripts-injection]: unregister content scripts before registration #26765

fix[dynamic-scripts-injection]: unregister content scripts before registration #26765

Conversation

hoxyq commented May 3, 2023

Summary

How did you test this change?

mondaychen left a comment

Choose a reason for hiding this comment

mondaychen May 3, 2023

Choose a reason for hiding this comment

hoxyq May 3, 2023

Choose a reason for hiding this comment