-
Notifications
You must be signed in to change notification settings - Fork 11
How React/Redux codebases integrate with Fronts/Clients #58
Comments
The first iteration of this RFC would be to agree on the description I made of each panel, that, without being argumentative yet. Could I get a review of at least one peer of each panel? |
I know the storage panel will eventually be part of our app panel (or whatever we call it) but I thought I would mention that it handles this in a completely different way. |
Thanks for doing this review Alex. That sounds correct for the console and reveals a lot of inconsistencies, that would be nice to unify as part of this work. |
thanks for doing this, the findings are really interesting. |
About performance-new: remember that the perf front is a root front, because we control the profiler from gecko's main process. Therefore do you think we'll need to have several targets for this one? About memory: this looks right from what I can see. |
Thanks for looking into this Alex! The description for the application panel looks good. For about:debugging, it is a bit different from the application panel.
We are definitely open to change this for more consistency. I will just share some of the motivations behind the current choices (and maybe others can point at better patterns to address them). One of the strong drives was to have similar looking data structures for all our debug targets so that we could build generic components to display them. Initially not all of our debug targets had fronts as well so using the actor id was the only consistent way to store an access point to the server. Finally we have a "client-wrapper" class to avoid calling the client directly from our application code, so that we can have a layer which is easy to mock. Open to any other solution (service container? commands?) that still preserves the ability to mock this easily in browser mochitests. (You asked whether we had documents about architecture. The only thing I have are:
|
Thanks Alex! I think the debugger does two smart things:
The redux actions receive the client via configureStore({
log: prefs.logging,
timing: isDevelopment(),
makeThunkArgs: (args, state) => ({ ...args, client })
}); Separating the
it("should add an expression", async () => {
const mockClient = { evaluateInFrame: async expression => `blah` };
const { dispatch, getState } = createStore(mockClient);
await dispatch(actions.addExpression("foo"));
expect(selectors.getExpressions(getState())).toEqual([{ input: "foo", value: "blah" ]);
});
function stepOver(): Promise<*> {
return threadClient.stepOver();
} When Brian added multi-target debugging we were able to update async function stepOver(thread: string): Promise<*> {
return lookupThreadClient(thread).stepOver();
} Keeping the RDP logic like threadClient in the client module was the primary reason we were able to make the debugger multi-target in a couple of weeks. addendum: chrome cdpThe debugger currently ~50% cdp, which makes it easy to compare CDP and RDP. Here is an example of chrome's async function setBreakpoint(location: SourceLocation, condition: string) {
const { breakpointId, serverLocation } = await debuggerAgent.setBreakpoint({
location: toServerLocation(location),
columnNumber: location.column
});
return {
id: breakpointId,
actualLocation: fromServerLocation(serverLocation) || location
};
} function setBreakpoint(
location: SourceLocation,
options: BreakpointOptions,
): Promise<BreakpointResult> {
const sourceThreadClient = sourceThreads[location.sourceId];
const sourceClient = sourceThreadClient.source({ actor: location.sourceId });
return sourceClient
.setBreakpoint(location, options)
.then(([{ actualLocation }, bpClient]) => {
actualLocation = createBreakpointLocation(location, actualLocation);
const id = makePendingLocationId(actualLocation);
bpClients[id] = bpClient;
return { id, actualLocation };
});
} The big difference is that Chrome's API accepts IDs and does not manage resource instances. |
Thanks Alex, looks pretty accurate for the Accessibility panel. |
Context: Fission.
For fission we will have to handle multiple targets at once. One per iframe in regular toolbox. One per process in the browser toolbox.
In order to handle this, typically in redux's actions, we will have to call the right target's front method. For now we had only one front, but now we will have many which will be specific to each resource.
I started looking into how we would handle this in all panels and saw that every panel was having a different way to interact with Fronts/Clients.
I think it is a good time to discuss between all panels and try to agree on a unified way to integrate a React/Redux codebase with Fronts and Clients.
Before suggesting anything I would like to first go over the codebase and correctly describe the current architecture of each panel using React and Redux.
I'm especially interested in describing:
Debugger
Click to expand
Source object type definition saved in store
An action calling a command
A command calling a thread client method
The reducer just pass the data as-is
A middleware is processing the front response
And maps it to a custom representation
SW registration front is saved in the redux store and used from a React component to hand it over to an action
The action calls a front method
Otherwise for every other resources we retrieve the front out of the ID, like here for addons:
Or for SW
https://searchfox.org/mozilla-central/rev/b29663c6c9c61b0bf29e8add490cbd6bad293a67/devtools/client/aboutdebugging-new/src/modules/client-wrapper.js#119-123
Or addons
Accessibility
Click to expand
An action receive the fronts from React props and call a front method. The response is passed to reducers.
Reducer process the response and maps it to a custom representation
performance-new
Click to expand
An action retrieve the target scoped front from the state and call a front method
Details on how we retrieve the front from the state
Application
Click to expand
React component calling a front method
initializer module call front method and then an action with the response without any mapping
Memory
Click to expand
React component calling an action with front coming from props
Action calling a front method
Netmonitor
Click to expand
Connector is calling an action with processed/aggregated data comping front Client responses
It does map data coming from RDP like here:
In is interesting to note a usage of target front usage
An action call the connector
The connector is calling the client method
console
Click to expand
JSTerm is calling a client method
WebConsoleFrame is calling a client method
WebConsoleOutputWrapper calls a client method
A React component is instantiating a client class
WebConsoleProxy listen to client events and call WebConsoleOutputWrapper
WebConsoleOutputWrapper dispatch actions (with some batching optimizations in the way)
The action is using a transformation to map the data to a custom representation
There is one action using the console client directly
https://searchfox.org/mozilla-central/rev/b29663c6c9c61b0bf29e8add490cbd6bad293a67/devtools/client/webconsole/reducers/autocomplete.js#45-84
The text was updated successfully, but these errors were encountered: