[DevTools] browser extension: improve script injection logic #26492

mondaychen · 2023-03-27T21:49:31Z

Summary

Drop extension support for Chrome / Edge <v102 since they have less than 0.1% usage (see data)
Improve script injection logic when possible so that the scripts injected by the extension are no longer shown in Network (which caused a lot of confusion in the past)

How did you test this change?

Built and tested locally, works as usual on Firefox.

For Chrome/Edge

Before:
Scripts shown in Network tab

After:
No scripts shown

hoxyq · 2023-03-28T15:40:18Z

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

+  } else if (request.payload?.tabId) {
+    const tabId = request.payload?.tabId;
+    // This is sent from the devtools page when it is ready for injecting the backend
+    if (request.payload.type === 'react-devtools-inject-backend') {


nit: Should we put react-devtools-inject-backend into constant?

I see its used in multiple places:

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

Line 192 in 41b4714

`window.postMessage({ source: 'react-devtools-inject-backend' }, '*');`,

react/packages/react-devtools-extensions/src/contentScripts/prepareInjection.js

Line 92 in 41b4714

case 'react-devtools-inject-backend':

Many event names/types are used in this way. I'm not against extracting them into a shared constant file as a better engineering work, but let's use another PR to take care of them all together.

The big difference imo between using a constant and literals in that a constant will get allocated at the module level and would weight the sum of all the literals plus some small management extra.
Literals on the other hand are only allocated when used and referenced (they may even get GCed like in case constant).

hoxyq · 2023-03-28T15:41:35Z

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

-            }
-          },
-        );
+        if (IS_CHROME | IS_EDGE) {


Is xor operator expected here?

Thanks for catching this!

Is it XOR operator? I think ^ is XOR operator.

Is it XOR operator? I think ^ is XOR operator.

Yeah, my bad, its a bitwise or

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

Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>

alvarlagerlof · 2023-03-29T05:12:14Z

Will Lighthouse still be able to find these?

Full list of changes (not everything included in changelog): * refactor[devtools]: copy to clipboard only on frontend side ([hoxyq](https://github.com/hoxyq) in [#26604](#26604)) * Provide icon to edge devtools. ([harrygz889](https://github.com/harrygz889) in [#26543](#26543)) * [BE] move shared types & constants to consolidated locations ([mondaychen](https://github.com/mondaychen) in [#26572](#26572)) * remove backend dependency from the global hook ([mondaychen](https://github.com/mondaychen) in [#26563](#26563)) * Replace deprecated `new-window` with `webContents.setWindowOpenHandler()` ([Willie-Boy](https://github.com/Willie-Boy) in [#26559](#26559)) * DevTools: Inline references to fiber flags ([acdlite](https://github.com/acdlite) in [#26542](#26542)) * refactor[devtools]: forbid editing class instances in props ([hoxyq](https://github.com/hoxyq) in [#26522](#26522)) * Move update scheduling to microtask ([acdlite](https://github.com/acdlite) in [#26512](#26512)) * Remove unnecessary CIRCLE_CI_API_TOKEN checks ([mondaychen](https://github.com/mondaychen) in [#26499](#26499)) * browser extension: improve script injection logic ([mondaychen](https://github.com/mondaychen) in [#26492](#26492)) * [flow] make Flow suppressions explicit on the error ([kassens](https://github.com/kassens) in [#26487](#26487))

Full list of changes (not everything included in changelog): * refactor[devtools]: copy to clipboard only on frontend side ([hoxyq](https://github.com/hoxyq) in [facebook#26604](facebook#26604)) * Provide icon to edge devtools. ([harrygz889](https://github.com/harrygz889) in [facebook#26543](facebook#26543)) * [BE] move shared types & constants to consolidated locations ([mondaychen](https://github.com/mondaychen) in [facebook#26572](facebook#26572)) * remove backend dependency from the global hook ([mondaychen](https://github.com/mondaychen) in [facebook#26563](facebook#26563)) * Replace deprecated `new-window` with `webContents.setWindowOpenHandler()` ([Willie-Boy](https://github.com/Willie-Boy) in [facebook#26559](facebook#26559)) * DevTools: Inline references to fiber flags ([acdlite](https://github.com/acdlite) in [facebook#26542](facebook#26542)) * refactor[devtools]: forbid editing class instances in props ([hoxyq](https://github.com/hoxyq) in [facebook#26522](facebook#26522)) * Move update scheduling to microtask ([acdlite](https://github.com/acdlite) in [facebook#26512](facebook#26512)) * Remove unnecessary CIRCLE_CI_API_TOKEN checks ([mondaychen](https://github.com/mondaychen) in [facebook#26499](facebook#26499)) * browser extension: improve script injection logic ([mondaychen](https://github.com/mondaychen) in [facebook#26492](facebook#26492)) * [flow] make Flow suppressions explicit on the error ([kassens](https://github.com/kassens) in [facebook#26487](facebook#26487))

…docked devtools window (#26665) bugfix for #26492 This bug would cause users unable to use the devtools (component tree empty). The else-if logic is broken when user switch to undocked devtools mode (separate window) because `sender.tab` would exist in that case. <img width="314" alt="image" src="https://user-images.githubusercontent.com/1001890/232930094-05a74445-9189-4d50-baf1-a0360b29ef7e.png"> Tested on Chrome with a local build

Full list of changes (not everything included in changelog): * refactor[devtools]: copy to clipboard only on frontend side ([hoxyq](https://github.com/hoxyq) in [facebook#26604](facebook#26604)) * Provide icon to edge devtools. ([harrygz889](https://github.com/harrygz889) in [facebook#26543](facebook#26543)) * [BE] move shared types & constants to consolidated locations ([mondaychen](https://github.com/mondaychen) in [facebook#26572](facebook#26572)) * remove backend dependency from the global hook ([mondaychen](https://github.com/mondaychen) in [facebook#26563](facebook#26563)) * Replace deprecated `new-window` with `webContents.setWindowOpenHandler()` ([Willie-Boy](https://github.com/Willie-Boy) in [facebook#26559](facebook#26559)) * DevTools: Inline references to fiber flags ([acdlite](https://github.com/acdlite) in [facebook#26542](facebook#26542)) * refactor[devtools]: forbid editing class instances in props ([hoxyq](https://github.com/hoxyq) in [facebook#26522](facebook#26522)) * Move update scheduling to microtask ([acdlite](https://github.com/acdlite) in [facebook#26512](facebook#26512)) * Remove unnecessary CIRCLE_CI_API_TOKEN checks ([mondaychen](https://github.com/mondaychen) in [facebook#26499](facebook#26499)) * browser extension: improve script injection logic ([mondaychen](https://github.com/mondaychen) in [facebook#26492](facebook#26492)) * [flow] make Flow suppressions explicit on the error ([kassens](https://github.com/kassens) in [facebook#26487](facebook#26487))

…docked devtools window (facebook#26665) bugfix for facebook#26492 This bug would cause users unable to use the devtools (component tree empty). The else-if logic is broken when user switch to undocked devtools mode (separate window) because `sender.tab` would exist in that case. <img width="314" alt="image" src="https://user-images.githubusercontent.com/1001890/232930094-05a74445-9189-4d50-baf1-a0360b29ef7e.png"> Tested on Chrome with a local build

## Summary - Drop extension support for Chrome / Edge <v102 since they have less than 0.1% usage ([see data](https://caniuse.com/usage-table)) - Improve script injection logic when possible so that the scripts injected by the extension are no longer shown in Network (which caused a lot of confusion in the past) ## How did you test this change? Built and tested locally, works as usual on Firefox. For Chrome/Edge **Before:** Scripts shown in Network tab <img width="1279" alt="Untitled 2" src="https://user-images.githubusercontent.com/1001890/228074363-1d00d503-d4b5-4339-8dd6-fd0467e36e3e.png"> **After:** No scripts shown <img width="1329" alt="image" src="https://user-images.githubusercontent.com/1001890/228074596-2084722b-bf3c-495e-a852-15f122233155.png"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com> DiffTrain build for commit f718199.

…docked devtools window (#26665) bugfix for #26492 This bug would cause users unable to use the devtools (component tree empty). The else-if logic is broken when user switch to undocked devtools mode (separate window) because `sender.tab` would exist in that case. <img width="314" alt="image" src="https://user-images.githubusercontent.com/1001890/232930094-05a74445-9189-4d50-baf1-a0360b29ef7e.png"> Tested on Chrome with a local build DiffTrain build for commit 96fd2fb.

[DevTools] improve script injection logic

53442d3

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

mondaychen added 2 commits March 28, 2023 10:33

fix lint

4b60980

fix lint

0904f8d

hoxyq reviewed Mar 28, 2023

View reviewed changes

packages/react-devtools-extensions/src/main.js Outdated Show resolved Hide resolved

mondaychen and others added 2 commits March 28, 2023 11:48

fix per comment

096811d

comment nit

002d855

Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>

hoxyq approved these changes Mar 28, 2023

View reviewed changes

mondaychen merged commit f718199 into facebook:main Mar 28, 2023

sammy-SC mentioned this pull request Apr 5, 2023

Use native scheduler if defined sammy-SC/react#1

Closed

hoxyq mentioned this pull request Apr 17, 2023

React DevTools 4.27.4 -> 4.27.5 #26637

Merged

mondaychen mentioned this pull request Apr 19, 2023

(patch)[DevTools] bug fix: backend injection logic not working for undocked devtools window #26665

Merged

Bartel-C8 mentioned this pull request Nov 27, 2023

[DevTools Bug]: Extension not loading for local files in Electron since 4.27.5 #27749

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DevTools] browser extension: improve script injection logic #26492

[DevTools] browser extension: improve script injection logic #26492

mondaychen commented Mar 27, 2023

hoxyq Mar 28, 2023

mondaychen Mar 28, 2023

incepter Mar 28, 2023

hoxyq Mar 28, 2023

mondaychen Mar 28, 2023

discover59 Mar 30, 2023

hoxyq Mar 30, 2023

alvarlagerlof commented Mar 29, 2023

[DevTools] browser extension: improve script injection logic #26492

[DevTools] browser extension: improve script injection logic #26492

Conversation

mondaychen commented Mar 27, 2023

Summary

How did you test this change?

hoxyq Mar 28, 2023

Choose a reason for hiding this comment

mondaychen Mar 28, 2023

Choose a reason for hiding this comment

incepter Mar 28, 2023

Choose a reason for hiding this comment

hoxyq Mar 28, 2023

Choose a reason for hiding this comment

mondaychen Mar 28, 2023

Choose a reason for hiding this comment

discover59 Mar 30, 2023

Choose a reason for hiding this comment

hoxyq Mar 30, 2023

Choose a reason for hiding this comment

alvarlagerlof commented Mar 29, 2023