From d4747567c99dddf31a203c4e75603a531d57ffa0 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Mon, 4 Aug 2025 11:05:32 -0400 Subject: [PATCH] Change withSize to accept PropsWithoutSize as its type parameter. This simplifies porting it to TypeScript. Also move some withSize calls to be within the explicitConnect call because that allows Flow to infer the props argument in more cases. I'm actually not sure why I have to manually pass $Diff in many cases still - TypeScript infers the parameter correctly and we can leave it out everywhere. --- src/components/network-chart/index.js | 64 ++++++++++--------- src/components/shared/WithSize.js | 28 +++----- src/components/shared/thread/ActivityGraph.js | 4 +- src/components/shared/thread/SampleGraph.js | 4 +- .../timeline/EmptyThreadIndicator.js | 4 +- src/components/timeline/FullTimeline.js | 2 +- src/components/timeline/Markers.js | 20 ++---- .../timeline/TrackBandwidthGraph.js | 2 +- .../timeline/TrackCustomMarkerGraph.js | 2 +- .../timeline/TrackEventDelayGraph.js | 2 +- src/components/timeline/TrackMemoryGraph.js | 2 +- src/components/timeline/TrackNetwork.js | 2 +- src/components/timeline/TrackPowerGraph.js | 2 +- .../timeline/TrackProcessCPUGraph.js | 2 +- src/components/timeline/TrackScreenshots.js | 2 +- src/components/timeline/TrackThread.js | 2 +- .../timeline/TrackVisualProgressGraph.js | 2 +- src/test/types/with-size.js | 11 +++- 18 files changed, 77 insertions(+), 80 deletions(-) diff --git a/src/components/network-chart/index.js b/src/components/network-chart/index.js index f9f3e4860c..132bfd065e 100644 --- a/src/components/network-chart/index.js +++ b/src/components/network-chart/index.js @@ -43,7 +43,8 @@ import './index.css'; const ROW_HEIGHT = 16; -// The SizeProps are injected by the WithSize higher order component. +type OwnProps = {||}; + type DispatchProps = {| +changeSelectedNetworkMarker: typeof changeSelectedNetworkMarker, +changeRightClickedMarker: typeof changeRightClickedMarker, @@ -62,9 +63,10 @@ type StateProps = {| +scrollToSelectionGeneration: number, |}; -type OwnProps = {| ...SizeProps |}; - -type Props = ConnectedProps; +type Props = {| + ...SizeProps, + ...ConnectedProps, +|}; class NetworkChartImpl extends React.PureComponent { _virtualListRef = React.createRef>(); @@ -368,33 +370,33 @@ class NetworkChartImpl extends React.PureComponent { * Wrap the component in the WithSize higher order component, as well as the redux * connected component. */ -const ConnectedComponent = explicitConnect( - { - mapStateToProps: (state) => ({ - markerIndexes: - selectedThreadSelectors.getSearchFilteredNetworkMarkerIndexes(state), - scrollToSelectionGeneration: getScrollToSelectionGeneration(state), - getMarker: selectedThreadSelectors.getMarkerGetter(state), - selectedNetworkMarkerIndex: - selectedThreadSelectors.getSelectedNetworkMarkerIndex(state), - rightClickedMarkerIndex: - selectedThreadSelectors.getRightClickedMarkerIndex(state), - hoveredMarkerIndexFromState: - selectedThreadSelectors.getHoveredMarkerIndex(state), - timeRange: getPreviewSelectionRange(state), - disableOverscan: getPreviewSelection(state).isModifying, - threadsKey: getSelectedThreadsKey(state), - }), - mapDispatchToProps: { - changeSelectedNetworkMarker, - changeRightClickedMarker, - changeHoveredMarker, - }, - component: NetworkChartImpl, - } -); - -export const NetworkChart = withSize(ConnectedComponent); +export const NetworkChart = explicitConnect< + OwnProps, + StateProps, + DispatchProps, +>({ + mapStateToProps: (state) => ({ + markerIndexes: + selectedThreadSelectors.getSearchFilteredNetworkMarkerIndexes(state), + scrollToSelectionGeneration: getScrollToSelectionGeneration(state), + getMarker: selectedThreadSelectors.getMarkerGetter(state), + selectedNetworkMarkerIndex: + selectedThreadSelectors.getSelectedNetworkMarkerIndex(state), + rightClickedMarkerIndex: + selectedThreadSelectors.getRightClickedMarkerIndex(state), + hoveredMarkerIndexFromState: + selectedThreadSelectors.getHoveredMarkerIndex(state), + timeRange: getPreviewSelectionRange(state), + disableOverscan: getPreviewSelection(state).isModifying, + threadsKey: getSelectedThreadsKey(state), + }), + mapDispatchToProps: { + changeSelectedNetworkMarker, + changeRightClickedMarker, + changeHoveredMarker, + }, + component: withSize(NetworkChartImpl), +}); /** * Our definition of markers does not currently have the ability to refine diff --git a/src/components/shared/WithSize.js b/src/components/shared/WithSize.js index a858194b6b..7240279266 100644 --- a/src/components/shared/WithSize.js +++ b/src/components/shared/WithSize.js @@ -15,6 +15,8 @@ type State = {| export type SizeProps = $ReadOnly; +export type PropsWithSize = {| ...Props, ...SizeProps |}; + /** * Wraps a React component and makes 'width' and 'height' available in the * wrapped component's props. These props start out at zero and are updated to @@ -24,26 +26,14 @@ export type SizeProps = $ReadOnly; * * Note that the props are *not* updated if the size of the element changes * for reasons other than a window resize. - * - * Usage: withSize must be used with explicit type arguments. - * - * Correct: withSize(ComponentClass) - * Incorrect: withSize(ComponentClass) */ -export function withSize< - // The SizeProps act as a bounds on the generic props. This ensures that the props - // that passed in take into account they are being given the width and height. - Props: $ReadOnly<{ ...SizeProps }>, ->(Wrapped: React.ComponentType): React.ComponentType< - // The component that is returned does not accept width and height parameters, as - // they are injected by this higher order component. - $ReadOnly<$Diff>, -> { - // An existential type in a generic is a bit tricky to remove. Perhaps this can - // use a hook instead. - // See: https://github.com/firefox-devtools/profiler/issues/3062 - // eslint-disable-next-line flowtype/no-existential-type - return class WithSizeWrapper extends React.PureComponent<*, State> { +export function withSize( + Wrapped: React.ComponentType> +): React.ComponentType { + return class WithSizeWrapper extends React.PureComponent< + Props, + State, + > { state = { width: 0, height: 0 }; _container: HTMLElement | null; diff --git a/src/components/shared/thread/ActivityGraph.js b/src/components/shared/thread/ActivityGraph.js index b541f1df29..465d00839b 100644 --- a/src/components/shared/thread/ActivityGraph.js +++ b/src/components/shared/thread/ActivityGraph.js @@ -199,4 +199,6 @@ class ThreadActivityGraphImpl extends React.PureComponent { } } -export const ThreadActivityGraph = withSize(ThreadActivityGraphImpl); +export const ThreadActivityGraph = withSize<$Diff>( + ThreadActivityGraphImpl +); diff --git a/src/components/shared/thread/SampleGraph.js b/src/components/shared/thread/SampleGraph.js index 533448256b..da7f530c7f 100644 --- a/src/components/shared/thread/SampleGraph.js +++ b/src/components/shared/thread/SampleGraph.js @@ -393,4 +393,6 @@ export class ThreadSampleGraphImpl extends PureComponent { } } -export const ThreadSampleGraph = withSize(ThreadSampleGraphImpl); +export const ThreadSampleGraph = withSize<$Diff>( + ThreadSampleGraphImpl +); diff --git a/src/components/timeline/EmptyThreadIndicator.js b/src/components/timeline/EmptyThreadIndicator.js index 733632f589..b26d31ea75 100644 --- a/src/components/timeline/EmptyThreadIndicator.js +++ b/src/components/timeline/EmptyThreadIndicator.js @@ -143,4 +143,6 @@ export function getIndicatorPositions(props: Props): {| return { startup, shutdown, emptyBufferStart }; } -export const EmptyThreadIndicator = withSize(EmptyThreadIndicatorImpl); +export const EmptyThreadIndicator = withSize<$Diff>( + EmptyThreadIndicatorImpl +); diff --git a/src/components/timeline/FullTimeline.js b/src/components/timeline/FullTimeline.js index 024162d14d..b50508dc16 100644 --- a/src/components/timeline/FullTimeline.js +++ b/src/components/timeline/FullTimeline.js @@ -222,5 +222,5 @@ export const FullTimeline = explicitConnect< changeGlobalTrackOrder, changeRightClickedTrack, }, - component: withSize(FullTimelineImpl), + component: withSize(FullTimelineImpl), }); diff --git a/src/components/timeline/Markers.js b/src/components/timeline/Markers.js index dda437aa9f..02ce6e5736 100644 --- a/src/components/timeline/Markers.js +++ b/src/components/timeline/Markers.js @@ -219,7 +219,7 @@ class TimelineMarkersCanvas extends React.PureComponent { this._requestedAnimationFrame = false; const c = this._canvas.current; if (c) { - timeCode('TimelineMarkersImplementation render', () => { + timeCode('TimelineMarkers render', () => { this.drawCanvas(c); }); } @@ -316,7 +316,7 @@ type State = { mouseY: CssPixels, }; -class TimelineMarkersImplementation extends React.PureComponent { +class TimelineMarkers extends React.PureComponent { state = { hoveredMarkerIndex: null, mouseDownMarker: null, @@ -505,12 +505,6 @@ class TimelineMarkersImplementation extends React.PureComponent { } } -/** - * Combine the base implementation of the TimelineMarkers with the - * WithSize component. - */ -export const TimelineMarkers = withSize(TimelineMarkersImplementation); - /** * Memoize the isSelected result of the markers since this is user multiple times. */ @@ -543,7 +537,7 @@ export const TimelineMarkersJank = explicitConnect< }; }, mapDispatchToProps: { changeRightClickedMarker }, - component: TimelineMarkers, + component: withSize(TimelineMarkers), }); /** @@ -572,7 +566,7 @@ export const TimelineMarkersOverview = explicitConnect< }; }, mapDispatchToProps: { changeRightClickedMarker }, - component: TimelineMarkers, + component: withSize(TimelineMarkers), }); /** @@ -598,7 +592,7 @@ export const TimelineMarkersFileIo = explicitConnect< }; }, mapDispatchToProps: { changeRightClickedMarker }, - component: TimelineMarkers, + component: withSize(TimelineMarkers), }); /** @@ -625,7 +619,7 @@ export const TimelineMarkersMemory = explicitConnect< }; }, mapDispatchToProps: { changeRightClickedMarker }, - component: TimelineMarkers, + component: withSize(TimelineMarkers), }); /** @@ -652,5 +646,5 @@ export const TimelineMarkersIPC = explicitConnect< }; }, mapDispatchToProps: { changeRightClickedMarker }, - component: TimelineMarkers, + component: withSize(TimelineMarkers), }); diff --git a/src/components/timeline/TrackBandwidthGraph.js b/src/components/timeline/TrackBandwidthGraph.js index 360b3daa7b..daf6ead991 100644 --- a/src/components/timeline/TrackBandwidthGraph.js +++ b/src/components/timeline/TrackBandwidthGraph.js @@ -711,5 +711,5 @@ export const TrackBandwidthGraph = explicitConnect< previewSelection: getPreviewSelection(state), }; }, - component: withSize(TrackBandwidthGraphImpl), + component: withSize(TrackBandwidthGraphImpl), }); diff --git a/src/components/timeline/TrackCustomMarkerGraph.js b/src/components/timeline/TrackCustomMarkerGraph.js index fb1d643989..501902dafb 100644 --- a/src/components/timeline/TrackCustomMarkerGraph.js +++ b/src/components/timeline/TrackCustomMarkerGraph.js @@ -631,5 +631,5 @@ export const TrackCustomMarkerGraph = explicitConnect< getMarker: selectors.getMarkerGetter(state), }; }, - component: withSize(TrackCustomMarkerGraphImpl), + component: withSize(TrackCustomMarkerGraphImpl), }); diff --git a/src/components/timeline/TrackEventDelayGraph.js b/src/components/timeline/TrackEventDelayGraph.js index f4c1610473..063f825400 100644 --- a/src/components/timeline/TrackEventDelayGraph.js +++ b/src/components/timeline/TrackEventDelayGraph.js @@ -380,5 +380,5 @@ export const TrackEventDelayGraph = explicitConnect< eventDelays: selectors.getProcessedEventDelays(state), }; }, - component: withSize(TrackEventDelayGraphImpl), + component: withSize(TrackEventDelayGraphImpl), }); diff --git a/src/components/timeline/TrackMemoryGraph.js b/src/components/timeline/TrackMemoryGraph.js index 9e02fa9697..257d28ff66 100644 --- a/src/components/timeline/TrackMemoryGraph.js +++ b/src/components/timeline/TrackMemoryGraph.js @@ -550,5 +550,5 @@ export const TrackMemoryGraph = explicitConnect< unfilteredSamplesRange: selectors.unfilteredSamplesRange(state), }; }, - component: withSize(TrackMemoryGraphImpl), + component: withSize(TrackMemoryGraphImpl), }); diff --git a/src/components/timeline/TrackNetwork.js b/src/components/timeline/TrackNetwork.js index a3319ef28a..85fe39f7b5 100644 --- a/src/components/timeline/TrackNetwork.js +++ b/src/components/timeline/TrackNetwork.js @@ -445,5 +445,5 @@ export const TrackNetwork = explicitConnect< changeSelectedNetworkMarker, changeHoveredMarker, }, - component: withSize(Network), + component: withSize(Network), }); diff --git a/src/components/timeline/TrackPowerGraph.js b/src/components/timeline/TrackPowerGraph.js index 1e52625862..3d99271882 100644 --- a/src/components/timeline/TrackPowerGraph.js +++ b/src/components/timeline/TrackPowerGraph.js @@ -575,5 +575,5 @@ export const TrackPowerGraph = explicitConnect< unfilteredSamplesRange: selectors.unfilteredSamplesRange(state), }; }, - component: withSize(TrackPowerGraphImpl), + component: withSize(TrackPowerGraphImpl), }); diff --git a/src/components/timeline/TrackProcessCPUGraph.js b/src/components/timeline/TrackProcessCPUGraph.js index 7ececacb0e..97c0f21d73 100644 --- a/src/components/timeline/TrackProcessCPUGraph.js +++ b/src/components/timeline/TrackProcessCPUGraph.js @@ -478,5 +478,5 @@ export const TrackProcessCPUGraph = explicitConnect< unfilteredSamplesRange: selectors.unfilteredSamplesRange(state), }; }, - component: withSize(TrackProcessCPUGraphImpl), + component: withSize(TrackProcessCPUGraphImpl), }); diff --git a/src/components/timeline/TrackScreenshots.js b/src/components/timeline/TrackScreenshots.js index ca69bd6757..8889528dee 100644 --- a/src/components/timeline/TrackScreenshots.js +++ b/src/components/timeline/TrackScreenshots.js @@ -211,7 +211,7 @@ export const TimelineTrackScreenshots = explicitConnect< mapDispatchToProps: { updatePreviewSelection, }, - component: withSize(Screenshots), + component: withSize(Screenshots), }); type HoverPreviewProps = {| diff --git a/src/components/timeline/TrackThread.js b/src/components/timeline/TrackThread.js index 85b2d6b43e..df81326dd1 100644 --- a/src/components/timeline/TrackThread.js +++ b/src/components/timeline/TrackThread.js @@ -384,5 +384,5 @@ export const TimelineTrackThread = explicitConnect< selectSelfCallNode, reportTrackThreadHeight, }, - component: withSize(TimelineTrackThreadImpl), + component: withSize(TimelineTrackThreadImpl), }); diff --git a/src/components/timeline/TrackVisualProgressGraph.js b/src/components/timeline/TrackVisualProgressGraph.js index ae9f23019d..3faea2481c 100644 --- a/src/components/timeline/TrackVisualProgressGraph.js +++ b/src/components/timeline/TrackVisualProgressGraph.js @@ -348,5 +348,5 @@ export const TrackVisualProgressGraph = explicitConnect< interval: getProfileInterval(state), }; }, - component: withSize(TrackVisualProgressGraphImpl), + component: withSize(TrackVisualProgressGraphImpl), }); diff --git a/src/test/types/with-size.js b/src/test/types/with-size.js index 06a39dd9b4..b8bf6173c9 100644 --- a/src/test/types/with-size.js +++ b/src/test/types/with-size.js @@ -48,7 +48,8 @@ const example2 = ( /> ); -const ExampleComponentWithSize = withSize(ExampleComponent); +const ExampleComponentWithSize = + withSize<$Diff>(ExampleComponent); // This it the correct use const exampleWithSize1 = ; @@ -79,5 +80,9 @@ class NoSizing extends React.PureComponent { } } -// $FlowExpectError - The component does not have sizing props. -const exampleNoSizing = withSize(NoSizing); +// This test no longer works. +// Not sure why Flow accepts NoSizing as a React.ComponentType> +// TypeScript catches this particular error, so this test will be re-enabled once +// we migrate. +// /*$*/FlowExpectError - The component does not have sizing props. +const exampleNoSizing = withSize(NoSizing);