React Component Dependencies and Behavior Un- or Underdeclared

[colin-grant-work]

In many cases, the React components defined here do not declare their data dependencies or behaviors clearly. For example:

theia-trace-extension/packages/react-components/src/trace-explorer/trace-explorer-time-range-data-widget.tsx

Lines 7 to 10 in 02ac259

	export interface ReactTimeRangeDataWidgetProps {
	id: string;
	title: string;
	}

From its props, that component gives absolutely no indication of what data it requires or what actions it can trigger, and although that's an extreme case, it reflects a general pattern.

A major contributor to the prevalence of that pattern in the code base in the widespread use of the global SignalManager instance. Because the SignalManager is accessed via the global signalManager getter, it has become the central clearinghouse for all kinds of often unrelated data and interactions in the application. That has a number of consequences. As already noted, it means that components that use the signal manager have undeclared dependencies and behavior: only by reading the code is it possible to determine what signals a given component subscribes to and what signals a given component causes the signal manager to emit. It also means that in many cases, there is no source of truth about the current state of the application: it's hoped that the right signals have been emitted and the right actions have been taken on the basis of those signals, but every component can do with those signals what they want, and constructing a canonical, application-wide state is impossible. A corollary is that many views maintain state redundantly: because there is no other source of truth, any view that wants to refer to the content of a past signal has to store that content itself. Thirdly, it makes it difficult to refactor the code to use different services or service implementations, because views combine the role of both view and controller: they expect to be able to fetch their own data in ways they don't always declare. This is clear in #990 where a number of cases have had to be retyped to TspClient | TspFrontendClient because views depended on a concrete implementation of a given service rather than declaring the kinds of data required and the kinds of actions needed. It has also been a hindrance to the reimplentation of the trace view functionality the VSCode Trace Extension, because different views in that extension reside in different JS execution contexts, and so the global SignalManager is no longer global, putting up a hurdle to reimplementing inter-view interactions, especially when those interactions are only discoverable by reading the whole code base.

Concretely:

• The global SignalManager should be replaced with services that combine signals with state maintenance (a better-defined controller layer than the current code offers).
• Views should avoid direct inter-view signaling and data fetching, and instead work through services, ideally accepting either service instances or action handlers as props.
• Views should favor props over state. In general, a given view's state should reflect data under its control and relevant only to it and its children, while data relevant at a higher level should be passed in from that higher level.