Add reload and profile to react-devtools-inline #27733

Jack-Works · 2023-11-21T12:47:43Z

Summary

This "attach" function is necessary to support the reload & profile function, but it is not accessible from the react-devtools-inline package.

How did you test this change?

I patched my node_modules to expose this function. It works for me.

hoxyq · 2023-11-21T16:58:27Z

This "attach" function is necessary to support the reload & profile function

Yes, but only if you do all the necessary steps for reloading and start profiling on the application side.

Is there a way for react-devtools-inline to control all of this flow? I believe there is more to it, inline version should set __REACT_DEVTOOLS_ATTACH__ global on the target to allow React DevTools hook inject before the app loads:
https://github.com/facebook/react/blob/main/packages/react-devtools-shared/src/hook.js#L350-L356

I haven't explored if this is currently blocked by something, but I would prefer this approach over exporting attach.

Jack-Works · 2023-11-22T01:25:18Z

Yes, but only if you do all the necessary steps for reloading and start profiling on the application side.

yes, I did that by setting sessionStorage.

Is there a way for react-devtools-inline to control all of this flow? I believe there is more to it, inline version should set REACT_DEVTOOLS_ATTACH global on the target to allow React DevTools hook inject before the app loads:

if react-devtools-inline detects sessionStorage and set attach on the global, it works for me but I'm not sure if it work for others.

hoxyq · 2023-11-22T15:20:14Z

if react-devtools-inline detects sessionStorage and set attach on the global, it works for me but I'm not sure if it work for others.

Can you test it with react-devtools-shell package, where we are mounting inline version of the DevTools? If that works, I think we can ship it.

I can help you with double-checking these changes a bit later.

Jack-Works · 2023-11-22T16:48:21Z

if react-devtools-inline detects sessionStorage and set attach on the global, it works for me but I'm not sure if it work for others.

Can you test it with react-devtools-shell package, where we are mounting inline version of the DevTools? If that works, I think we can ship it.

I can help you with double-checking these changes a bit later.

I don't know what I need to test, it just re-exports a function...

if you're interested, this is the workaround I currently using. If this function is exported, I can do it myself instead of patch package.

hoxyq · 2023-11-22T18:14:17Z

I don't know what I need to test, it just re-exports a function...

Sorry if I wasn't clear. Instead of just re-exporting attach here, can you try setting __REACT_DEVTOOLS_ATTACH__ global on the target and test this solution in react-devtools-shell?

if you're interested, this is the workaround I currently using. If this function is exported, I can do it myself instead of patch package.

I think we should have this patch in DevTools, this is the solution I am looking for. If everything is fine, then just update the PR, I can test it later and have it merged.

Jack-Works · 2023-11-23T06:07:35Z

hi @hoxyq please take a look!

hoxyq

Thanks for working on this. You are on the right track, but the current implementation doesn't solve the existing problem.

Some definitions:

react-devtools-shell is a local-only package, which hosts a testing shell for DevTools. It uses inline version of the React DevTools.
react-devtools-shared contains all shared code across core / extension / inline versions of the DevTools.

What we want to achieve:

In inline version of the DevTools user should be able to click on Reload and profile button and start profiling.

With just exporting attach, users won't have this button available to them, it won't be visible, because it relies on supportsReloadAndProfile flag.

What I expect from changes (essentially, to look the same as on other surfaces):

User should be able to click on the 'Reload and profile' button.
The page reloads and profiling button is highlighted in red (profiling in progress).
User can click on this button to stop profiling and he will get a report.

You can test these changes in react-devtools-shell, which uses the inline version. I don't expect you to update anything in the shell itself, it is just a testing shell.

Some useful links:

The place where we create Store for inline version of the DevTools, where you can provide supportsReloadAndProfile flag -

react/packages/react-devtools-inline/src/frontend.js

Lines 27 to 34 in 77ec618

    
             return new Store(bridge, { 
        
               checkBridgeProtocolCompatibility: true, 
        
               supportsTraceUpdates: true, 
        
               supportsTimeline: true, 
        
               supportsNativeInspection: true, 
        
               ...config, 
        
             }); 
        
           }

How we define __REACT_DEVTOOLS_ATTACH__ for extension version of the DevTools -

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

Lines 15 to 33 in dd53658

    
           if ( 
        
             sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' && 
        
             !window.hasOwnProperty('__REACT_DEVTOOLS_ATTACH__') 
        
           ) { 
        
             Object.defineProperty( 
        
               window, 
        
               '__REACT_DEVTOOLS_ATTACH__', 
        
               ({ 
        
                 enumerable: false, 
        
                 // This property needs to be configurable to allow third-party integrations 
        
                 // to attach their own renderer. Note that using third-party integrations 
        
                 // is not officially supported. Use at your own risk. 
        
                 configurable: true, 
        
                 get() { 
        
                   return attach; 
        
                 }, 
        
               }: Object), 
        
             ); 
        
           }

The backend should listen to 'reloadAndProfile' event. This is how its implemented in the extension -

react/packages/react-devtools-shared/src/backend/agent.js

Lines 583 to 595 in 77ec618

    
           reloadAndProfile: (recordChangeDescriptions: boolean) => void = 
        
             recordChangeDescriptions => { 
        
               sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true'); 
        
               sessionStorageSetItem( 
        
                 SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, 
        
                 recordChangeDescriptions ? 'true' : 'false', 
        
               ); 
        
               // This code path should only be hit if the shell has explicitly told the Store that it supports profiling. 
        
               // In that case, the shell must also listen for this specific message to know when it needs to reload the app. 
        
               // The agent can't do this in a way that is renderer agnostic. 
        
               this._bridge.send('reloadAppForProfiling'); 
        
             };

It sends an 'reloadAppForProfiling' to the frontend to reload the page. You won't need that for inline version, because you probably can just reload the page with contentWindow.parent.location.reload().

Jack-Works · 2023-11-24T08:04:05Z

That's interesting but contains more changes. I will try to do it later.

Jack-Works · 2023-11-24T18:55:18Z

hi @hoxyq please take a look thanks. I believe this package is experimental so I bring some breaking changes to support reload and profile.

hoxyq

We probably don't need to change anything in react-devtools-shell, why changing the environment?

You need to do 3 things:

Provide supportsReloadAndProfile flag with true value. (What you almost doing)
Inject __REACT_DEVTOOLS_ATTACH__, what you are doing.
Backend should listen for 'reloadAndProfile' event, after which you need to set all necessary flags to sessions storage and reload the whole document. You don't need to send 'reloadAppForProfiling' to the frontend, because the frontend will be reloaded together with the whole document (frontend is in an iframe inside this document). You need to reload what you are debugging, so backend and frontend of the DevTools will be reloaded with it.

Thanks again for working on this, I can help with implementing these probably next week, if you feeling stuck.

hoxyq · 2023-11-28T18:47:22Z

packages/react-devtools-inline/src/frontend.js

+    createBridge: customCreateBridge,
+    createStore: customCreateStore,


Why do you need these custom functions?

In our use case, I need to unload and reload the DevTools without refreshing the whole page. What I do in this PR is reloading the app iframe only, and destroy then create a new store, so the main page won't need to refresh.

Jack-Works · 2023-11-29T03:48:49Z

I changed it according to your review, but it became less fit our use case (I need to manually destroy and create store & dev tools because I cannot refresh the page).

hoxyq · 2023-11-29T12:01:20Z

I need to manually destroy and create store & dev tools because I cannot refresh the page

Why not just reloading the whole page? Backend is injected in the page's context and it should be able to reload it with window.location.reload

Jack-Works · 2023-11-29T15:21:44Z

I need to manually destroy and create store & dev tools because I cannot refresh the page

Why not just reloading the whole page? Backend is injected in the page's context and it should be able to reload it with window.location.reload

I cannot refresh the front-end (DevTools). Backend (app) is ok

hoxyq · 2023-11-29T18:16:52Z

I need to manually destroy and create store & dev tools because I cannot refresh the page

Why not just reloading the whole page? Backend is injected in the page's context and it should be able to reload it with window.location.reload

I cannot refresh the front-end (DevTools). Backend (app) is ok

Can you share more on your setup? Is frontend running in a different context / page?

Jack-Works · 2023-11-30T06:33:47Z

Can you share more on your setup? Is frontend running in a different context / page?

Yes. Our setup is a Web Extension, therefore it cannot use the official React Devtools (web extensions cannot debug each other). I have integrated react-devtools-inline and made it work like how react-devtools-extension works.

hoxyq · 2023-12-07T11:42:35Z

Can you share more on your setup? Is frontend running in a different context / page?

Yes. Our setup is a Web Extension, therefore it cannot use the official React Devtools (web extensions cannot debug each other). I have integrated react-devtools-inline and made it work like how react-devtools-extension works.

Why have you chosen inline version of the DevTools and not building something on top of core package?

Anyways, with this setup it will require more than what I've mentioned before and yes, you would need to reload the frontend as well or make it resetting the store.

The scope of this feature is much bigger now, I can help with reviewing the changes, but can't really spend much time digging into this right now, as I am going on PTO next week.

My requirements would be:

No changes in react-devtools-shell, since this is the primary way of using react-devtools-inline, when both frontend and backend are in the same document, but in different frames.
Remove reload callback from the frontend. Can you try making it similar as much as possible to the implementation of this feature for the browser extension?

Jack-Works · 2023-12-08T07:23:46Z

Why have you chosen inline version of the DevTools and not building something on top of core package?

I'll try that. I cannot recall if I tried the core package before. I'll back to this PR after I try that.

Jack-Works · 2023-12-22T06:44:51Z

Why have you chosen inline version of the DevTools and not building something on top of core package?

Hi @hoxyq, I looked at the core package and found it impossible to use in our environment.

That package is too opinioned, the backend can only run in NodeJS, not the browser.

Here is what I found:

core/backend uses WebSocket to connect the frontend, but we cannot create a WebSocket server inside the browser. It's possible to duck-type a fake WebSocket interface but it is annoying.
core/standalone cannot run in the browser. It creates a WebSocket server and spawns new processes to open an editor.

I think my current changes are necessary to let it work in our environment, but I think it would be better to publish react-devtools-shared on npm directly so I don't need to hack around the inline package.

hoxyq · 2024-01-09T13:20:11Z

Why have you chosen inline version of the DevTools and not building something on top of core package?

Hi @hoxyq, I looked at the core package and found it impossible to use in our environment.

That package is too opinioned, the backend can only run in NodeJS, not the browser.

Here is what I found:

core/backend uses WebSocket to connect the frontend, but we cannot create a WebSocket server inside the browser. It's possible to duck-type a fake WebSocket interface but it is annoying.

core/standalone cannot run in the browser. It creates a WebSocket server and spawns new processes to open an editor.

I think my current changes are necessary to let it work in our environment, but I think it would be better to publish react-devtools-shared on npm directly so I don't need to hack around the inline package.

Thanks for the explanation. I will be back in a few days, hopefully will get you unblocked on this.

hoxyq · 2024-02-20T14:25:38Z

Similar attempt: #27978

github-actions · 2024-05-20T15:03:25Z

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

Jack-Works · 2024-05-20T16:40:21Z

bump

facebook-github-bot added the CLA Signed label Nov 21, 2023

Reexport attach from react-devtools-shared

f1ea6ac

Jack-Works force-pushed the patch-1 branch from 3529846 to 4601823 Compare November 23, 2023 05:48

hoxyq reviewed Nov 23, 2023

View reviewed changes

feat: support reload and profile

1639d66

Jack-Works force-pushed the patch-1 branch from 7f612d4 to 1639d66 Compare November 24, 2023 18:53

Jack-Works changed the title ~~Reexport attach from react-devtools-shared~~ Add reload and profile to react-devtools-inline Nov 25, 2023

hoxyq requested changes Nov 28, 2023

View reviewed changes

Jack-Works added 2 commits November 29, 2023 11:43

review

b978b90

fix: error

7cbb104

fix: ci

ecd1a47

github-actions bot added the Resolution: Stale Automatically closed due to inactivity label May 20, 2024

github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label May 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add reload and profile to react-devtools-inline #27733

Add reload and profile to react-devtools-inline #27733

Jack-Works commented Nov 21, 2023

hoxyq commented Nov 21, 2023

Jack-Works commented Nov 22, 2023

hoxyq commented Nov 22, 2023

Jack-Works commented Nov 22, 2023

hoxyq commented Nov 22, 2023

Jack-Works commented Nov 23, 2023

hoxyq left a comment •

edited

Jack-Works commented Nov 24, 2023

Jack-Works commented Nov 24, 2023

hoxyq left a comment

hoxyq Nov 28, 2023

Jack-Works Nov 29, 2023

Jack-Works commented Nov 29, 2023

hoxyq commented Nov 29, 2023 •

edited

Jack-Works commented Nov 29, 2023

hoxyq commented Nov 29, 2023

Jack-Works commented Nov 30, 2023

hoxyq commented Dec 7, 2023

Jack-Works commented Dec 8, 2023

Jack-Works commented Dec 22, 2023

hoxyq commented Jan 9, 2024

hoxyq commented Feb 20, 2024

github-actions bot commented May 20, 2024

Jack-Works commented May 20, 2024

	return new Store(bridge, {
	checkBridgeProtocolCompatibility: true,
	supportsTraceUpdates: true,
	supportsTimeline: true,
	supportsNativeInspection: true,
	...config,
	});
	}

	if (
	sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true' &&
	!window.hasOwnProperty('__REACT_DEVTOOLS_ATTACH__')
	) {
	Object.defineProperty(
	window,
	'__REACT_DEVTOOLS_ATTACH__',
	({
	enumerable: false,
	// This property needs to be configurable to allow third-party integrations
	// to attach their own renderer. Note that using third-party integrations
	// is not officially supported. Use at your own risk.
	configurable: true,
	get() {
	return attach;
	},
	}: Object),
	);
	}

	reloadAndProfile: (recordChangeDescriptions: boolean) => void =
	recordChangeDescriptions => {
	sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true');
	sessionStorageSetItem(
	SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY,
	recordChangeDescriptions ? 'true' : 'false',
	);

	// This code path should only be hit if the shell has explicitly told the Store that it supports profiling.
	// In that case, the shell must also listen for this specific message to know when it needs to reload the app.
	// The agent can't do this in a way that is renderer agnostic.
	this._bridge.send('reloadAppForProfiling');
	};

		createBridge: customCreateBridge,
		createStore: customCreateStore,

Add reload and profile to react-devtools-inline #27733

Are you sure you want to change the base?

Add reload and profile to react-devtools-inline #27733

Conversation

Jack-Works commented Nov 21, 2023

Summary

How did you test this change?

hoxyq commented Nov 21, 2023

Jack-Works commented Nov 22, 2023

hoxyq commented Nov 22, 2023

Jack-Works commented Nov 22, 2023

hoxyq commented Nov 22, 2023

Jack-Works commented Nov 23, 2023

hoxyq left a comment • edited

Choose a reason for hiding this comment

Jack-Works commented Nov 24, 2023

Jack-Works commented Nov 24, 2023

hoxyq left a comment

Choose a reason for hiding this comment

hoxyq Nov 28, 2023

Choose a reason for hiding this comment

Jack-Works Nov 29, 2023

Choose a reason for hiding this comment

Jack-Works commented Nov 29, 2023

hoxyq commented Nov 29, 2023 • edited

Jack-Works commented Nov 29, 2023

hoxyq commented Nov 29, 2023

Jack-Works commented Nov 30, 2023

hoxyq commented Dec 7, 2023

Jack-Works commented Dec 8, 2023

Jack-Works commented Dec 22, 2023

hoxyq commented Jan 9, 2024

hoxyq commented Feb 20, 2024

github-actions bot commented May 20, 2024

Jack-Works commented May 20, 2024

hoxyq left a comment •

edited

hoxyq commented Nov 29, 2023 •

edited