feat[react-devtools]: add settings to global hook object

[hoxyq]

Right now we are patching console 2 times: when hook is installed (before page is loaded) and when backend is connected. Because of this, even if user had appendComponentStack setting enabled, all emitted error and warning logs are not going to have component stacks appended. They also won't have component stacks appended retroactively when user opens browser DevTools (this is when frontend is initialized and connects to backend).

This behavior adds potential race conditions with LogBox in React Native, and also unpredictable to the user, because in order to get component stacks logged you have to open browser DevTools, but by the time you do it, error or warning log was already emitted.

To solve this, we are going to only patch console in the hook object, because it is guaranteed to load even before React. Settings are going to be synchronized with the hook via Bridge, and React DevTools Backend Host (React Native or browser extension shell) will be responsible for persisting these settings across the session, this is going to be implemented in a separate PR.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Sep 16, 2024 7:03am

[hoxyq]

This will be a successor of ConsolePatchSettings, the only difference is browserTheme field, but looks like it is not used. I will do it later in this stack.

[eps1lon]

even if user had appendComponentStack setting enabled, all emitted error and warning logs are not going to have component stacks appended

This is still the case with this change? It seemed fixed on one attempt but now I can't get it to work. console.error calls are still missing component stacks after I opened browser devtools. Only console.error calls after opened devtools have the stacks appended. Calls before I open devtools are also missed in the "show inline errors" display so maybe something else is not working?

[hoxyq]

even if user had appendComponentStack setting enabled, all emitted error and warning logs are not going to have component stacks appended

Only console.error calls after opened devtools have the stacks appended.

This is exactly what I am trying to fix in this stack of PRs. This is still the case for all PRs published by now. I am still working on it, and this will be fixed later in one of the PRs at the top of the stack.

[hoxyq]

Re-requesting reviews, since I've made some changes to the stack

[vzaidman]

let me make sure I understand- before you persist these settings, in this diff, you use the default settings in the hook, until the backend sends new settings as an event. correct?

[hoxyq]

let me make sure I understand- before you persist these settings, in this diff, you use the default settings in the hook, until the backend sends new settings as an event. correct?

Yes. This PR contains just a high-level API of how settings injection is going to look like. Third parties can pass an object or a Promise, which resolves with the object. If this Promise rejects, then we don't patch the console at all. If undefined or null is passed, then we inject default settings.

Persisting for browser extension settings will be added in #30636.

[hoxyq]

For context, we have to allow passing Promise as an option for settings, because we should not delay hook installation by waiting for settings to be fetched. This could result into global hook being installed after React is loaded on the page, and RDT won't work.

[vzaidman]

For context, we have to allow passing Promise as an option for settings, because we should not delay hook installation by waiting for settings to be fetched. This could result into global hook being installed after React is loaded on the page, and RDT won't work.

In what situation a promise is passed? Can't we always set the settings to default and only update them if a non-promise is passed?

[hoxyq]

For context, we have to allow passing Promise as an option for settings, because we should not delay hook installation by waiting for settings to be fetched. This could result into global hook being installed after React is loaded on the page, and RDT won't work.

In what situation a promise is passed? Can't we always set the settings to default and only update them if a non-promise is passed?

Promise will be passed for browser extension, for example. We are patching the console after the settings are initialized, so that we don't enable some features which were explicitly disabled by the user previously.

If we would await for settings, then this will delay the hook installation. As I said previously, this may result into global hook being installed after React is loaded on the page. React won't wait for RDT global hook to be installed. Also, we can't delay loading React on the page from the browser extension context, and I don't see reasons why we should.

[vzaidman]

What about instead of passing a promise/object as settings, just setHookSettings whenever you are ready to set them?

[hoxyq]

What about instead of passing a promise/object as settings, just setHookSettings whenever you are ready to set them?

This adds a race condition with changing settings in the UI. We have to be ready that something can override the settings in the middle of the session.

One significant disadvantage of this approach is that we never know if something is going to inject settings or not: there are no guarantees that this setHookSettings will be ever called. With the current approach, if nothing is passed, we patch the console with default values.

[eps1lon]

Is this typed or is it easy to forget to add/remove fields here?

[hoxyq]

It is typed, and will be updated to a shorter form in 248ddba