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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: refactored devtools browser extension scripts to improve port management and service worker lifetime #27215

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Aug 11, 2023

Fixes #27119, #27185.

Fixed:

  • React DevTools now works as expected when user performs in-tab navigation, previously it was just stuck.
Screen.Recording.2023-08-11.at.15.45.12.mov
  • When user closes browser DevTools panel, we now do some cleanup to disconnect ports and emit shutdown event for bridge. This should fix the issue with registering duplicated fibers with the same id in Store.

Changed:

  • We reconnect proxy port once in 25 seconds, in order to keep service worker alive.
  • Instead of unregistering dynamically injected content scripts, wen now get list of already registered scripts and filter them out from scripts that we want to inject again, see dynamicallyInjectContentScripts.js.
  • Split main.js and background.js into multiple files.

Tested on Chromium and Firefox browsers.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 11, 2023
@hoxyq hoxyq force-pushed the devtools/refactor-extension-scripts branch from 4b051b4 to eca432c Compare August 11, 2023 18:10
@hoxyq hoxyq marked this pull request as ready for review August 11, 2023 18:23
@hoxyq hoxyq force-pushed the devtools/refactor-extension-scripts branch from eca432c to 10836bc Compare August 29, 2023 10:11
@hoxyq hoxyq merged commit 8fbd307 into facebook:main Aug 29, 2023
@hoxyq hoxyq deleted the devtools/refactor-extension-scripts branch August 29, 2023 11:09
hoxyq added a commit that referenced this pull request Aug 29, 2023
List of changes:
* refactor: refactored devtools browser extension scripts to improve
port management and service worker lifetime
([hoxyq](https://github.com/hoxyq) in
[#27215](#27215))
* refactor[devtools/extension]: minify production builds to strip
comments ([hoxyq](https://github.com/hoxyq) in
[#27304](#27304))
* fix[devtools]: allow element updates polling only if bridge is alive
([hoxyq](https://github.com/hoxyq) in
[#27067](#27067))
* refactor: resolve browser via env variables based on build rather than
user agent ([hoxyq](https://github.com/hoxyq) in
[#27179](#27179))
* fix[devtools/updateFiberRecursively]: mount suspense fallback set in
timed out case ([hoxyq](https://github.com/hoxyq) in
[#27147](#27147))
* Feat:-Added open in editor to appear by default
([Biki-das](https://github.com/Biki-das) in
[#26949](#26949))
* fix[devtools/inspect]: null check memoized props before trying to call
hasOwnProperty ([hoxyq](https://github.com/hoxyq) in
[#27057](#27057))
* rename SuspenseList export to unstable_SuspenseList
([noahlemen](https://github.com/noahlemen) in
[#27061](#27061))
hoxyq added a commit that referenced this pull request Aug 30, 2023
… loaded (#27316)

This is mostly hotfix for #27215.

Contains 3 fixes:
- Handle cases when `react` is not loaded yet and user performs in-tab
navigation. Previously, because of the uncleared interval we would try
to mount DevTools twice, resulting into multiple errors.
- Handle case when extension port disconnected (probably by the browser
or just due to its lifetime)
- Removed duplicate `render()` call on line 327
hoxyq added a commit that referenced this pull request Sep 14, 2023
…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.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…rt management and service worker lifetime (facebook#27215)

Fixes facebook#27119,
facebook#27185.

Fixed:
- React DevTools now works as expected when user performs in-tab
navigation, previously it was just stuck.


https://github.com/facebook/react/assets/28902667/b11c5f84-7155-47a5-8b5a-7e90baca5347

- When user closes browser DevTools panel, we now do some cleanup to
disconnect ports and emit shutdown event for bridge. This should fix the
issue with registering duplicated fibers with the same id in Store.

Changed:
- We reconnect proxy port once in 25 seconds, in order to [keep service
worker
alive](https://developer.chrome.com/docs/extensions/whatsnew/#m110-sw-idle).
- Instead of unregistering dynamically injected content scripts, wen now
get list of already registered scripts and filter them out from scripts
that we want to inject again, see dynamicallyInjectContentScripts.js.
- Split `main.js` and `background.js` into multiple files.

Tested on Chromium and Firefox browsers.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
List of changes:
* refactor: refactored devtools browser extension scripts to improve
port management and service worker lifetime
([hoxyq](https://github.com/hoxyq) in
[facebook#27215](facebook#27215))
* refactor[devtools/extension]: minify production builds to strip
comments ([hoxyq](https://github.com/hoxyq) in
[facebook#27304](facebook#27304))
* fix[devtools]: allow element updates polling only if bridge is alive
([hoxyq](https://github.com/hoxyq) in
[facebook#27067](facebook#27067))
* refactor: resolve browser via env variables based on build rather than
user agent ([hoxyq](https://github.com/hoxyq) in
[facebook#27179](facebook#27179))
* fix[devtools/updateFiberRecursively]: mount suspense fallback set in
timed out case ([hoxyq](https://github.com/hoxyq) in
[facebook#27147](facebook#27147))
* Feat:-Added open in editor to appear by default
([Biki-das](https://github.com/Biki-das) in
[facebook#26949](facebook#26949))
* fix[devtools/inspect]: null check memoized props before trying to call
hasOwnProperty ([hoxyq](https://github.com/hoxyq) in
[facebook#27057](facebook#27057))
* rename SuspenseList export to unstable_SuspenseList
([noahlemen](https://github.com/noahlemen) in
[facebook#27061](facebook#27061))
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
… loaded (facebook#27316)

This is mostly hotfix for facebook#27215.

Contains 3 fixes:
- Handle cases when `react` is not loaded yet and user performs in-tab
navigation. Previously, because of the uncleared interval we would try
to mount DevTools twice, resulting into multiple errors.
- Handle case when extension port disconnected (probably by the browser
or just due to its lifetime)
- Removed duplicate `render()` call on line 327
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…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.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…rt management and service worker lifetime (#27215)

Fixes #27119,
#27185.

Fixed:
- React DevTools now works as expected when user performs in-tab
navigation, previously it was just stuck.

https://github.com/facebook/react/assets/28902667/b11c5f84-7155-47a5-8b5a-7e90baca5347

- When user closes browser DevTools panel, we now do some cleanup to
disconnect ports and emit shutdown event for bridge. This should fix the
issue with registering duplicated fibers with the same id in Store.

Changed:
- We reconnect proxy port once in 25 seconds, in order to [keep service
worker
alive](https://developer.chrome.com/docs/extensions/whatsnew/#m110-sw-idle).
- Instead of unregistering dynamically injected content scripts, wen now
get list of already registered scripts and filter them out from scripts
that we want to inject again, see dynamicallyInjectContentScripts.js.
- Split `main.js` and `background.js` into multiple files.

Tested on Chromium and Firefox browsers.

DiffTrain build for commit 8fbd307.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

[DevTools Bug]: Chrome extension gets disconnected from the page after 30sec of inactivity
2 participants