Skip to content

Update hooks to accepts Props as arguments which work better with renderHook() #106499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ryan953 wants to merge 10 commits into from
Closed

Update hooks to accepts Props as arguments which work better with renderHook() #106499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
85f92fe
Refactor useTourReducer to accept a props obj, instead of N args
ryan953 Jan 17, 2026
8814195
Refactor useDispatchingReducer to accept a props obj, instead of N args
ryan953 Jan 17, 2026
aeb7259
Refactor useMemoWithPrevious to accept a props obj, instead of N args
ryan953 Jan 17, 2026
088ddce
Refactor useUndoableReducer to accept a props obj, instead of N args
ryan953 Jan 17, 2026
b531d1b
Refactor useCopyIssueDetails to accept a props obj, instead of N args
ryan953 Jan 17, 2026
98d7a25
Refactor useReplaySummary to accept a props obj, instead of N args
ryan953 Jan 17, 2026
addd0d2
Refactor useEventColumns to accept a props obj, instead of N args
ryan953 Jan 17, 2026
310bb2c
Refactor useStreamingTimeseriesResult to accept a props obj, instead …
ryan953 Jan 17, 2026
5c1fd2f
Refactor make a testable version of useFetchEventsTimeSeries to play …
ryan953 Jan 18, 2026
a2d42d1
pass props into rerender()
ryan953 Jan 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 30 additions & 31 deletions static/app/components/profiling/flamegraph/continuousFlamegraph.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,8 +590,8 @@ export function ContinuousFlamegraph(): ReactElement {
return new FlamegraphCanvas(memoryChartCanvasRef, vec2.fromValues(0, 0));
}, [memoryChartCanvasRef]);

const flamegraphView = useMemoWithPrevious<CanvasView<FlamegraphModel> | null>(
previousView => {
const flamegraphView = useMemoWithPrevious<CanvasView<FlamegraphModel> | null>({
factory: previousView => {
if (!flamegraphCanvas) {
return null;
}
Expand Down Expand Up @@ -686,19 +686,18 @@ export function ContinuousFlamegraph(): ReactElement {
},

// We skip position.view dependency because it will go into an infinite loop
// eslint-disable-next-line react-hooks/exhaustive-deps
[
deps: [
flamegraph,
flamegraphCanvas,
flamegraphTheme,
profile,
segment,
configSpaceQueryParam,
]
);
],
});

const uiFramesView = useMemoWithPrevious<CanvasView<UIFrames> | null>(
_previousView => {
const uiFramesView = useMemoWithPrevious<CanvasView<UIFrames> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !uiFrames) {
return null;
}
Expand All @@ -724,18 +723,18 @@ export function ContinuousFlamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
flamegraph,
uiFrames,
profile,
configSpaceQueryParam,
]
);
],
});

const batteryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>(
_previousView => {
const batteryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !batteryChart || !batteryChartCanvas) {
return null;
}
Expand Down Expand Up @@ -768,19 +767,19 @@ export function ContinuousFlamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
batteryChart,
uiFrames.minFrameDuration,
batteryChartCanvas,
profile,
configSpaceQueryParam,
]
);
],
});

const cpuChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>(
_previousView => {
const cpuChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !CPUChart || !cpuChartCanvas) {
return null;
}
Expand Down Expand Up @@ -813,19 +812,19 @@ export function ContinuousFlamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
CPUChart,
uiFrames.minFrameDuration,
cpuChartCanvas,
profile,
configSpaceQueryParam,
]
);
],
});

const memoryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>(
_previousView => {
const memoryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !memoryChart || !memoryChartCanvas) {
return null;
}
Expand Down Expand Up @@ -858,19 +857,19 @@ export function ContinuousFlamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
memoryChart,
uiFrames.minFrameDuration,
memoryChartCanvas,
profile,
configSpaceQueryParam,
]
);
],
});

const spansView = useMemoWithPrevious<CanvasView<SpanChart> | null>(
_previousView => {
const spansView = useMemoWithPrevious<CanvasView<SpanChart> | null>({
factory: _previousView => {
if (!spansCanvas || !spanChart || !flamegraphView) {
return null;
}
Expand Down Expand Up @@ -899,16 +898,16 @@ export function ContinuousFlamegraph(): ReactElement {

return newView;
},
[
deps: [
spanChart,
spansCanvas,
flamegraphView,
flamegraphTheme.SIZES,
profileTimestamp,
configSpaceQueryParam,
segment,
]
);
],
});

// We want to make sure that the views have the same min zoom levels so that
// if you wheel zoom on one, the other one will also zoom to the same level of detail.
Expand Down
62 changes: 34 additions & 28 deletions static/app/components/profiling/flamegraph/flamegraph.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,8 +570,8 @@ function Flamegraph(): ReactElement {
return new FlamegraphCanvas(memoryChartCanvasRef, vec2.fromValues(0, 0));
}, [memoryChartCanvasRef]);

const flamegraphView = useMemoWithPrevious<CanvasView<FlamegraphModel> | null>(
previousView => {
const flamegraphView = useMemoWithPrevious<CanvasView<FlamegraphModel> | null>({
factory: previousView => {
if (!flamegraphCanvas) {
return null;
}
Expand Down Expand Up @@ -659,12 +659,12 @@ function Flamegraph(): ReactElement {
},

// We skip position.view dependency because it will go into an infinite loop
// eslint-disable-next-line react-hooks/exhaustive-deps
[flamegraph, flamegraphCanvas, flamegraphTheme, profileOffsetFromTransaction]
);

const uiFramesView = useMemoWithPrevious<CanvasView<UIFrames> | null>(
_previousView => {
deps: [flamegraph, flamegraphCanvas, flamegraphTheme, profileOffsetFromTransaction],
});

const uiFramesView = useMemoWithPrevious<CanvasView<UIFrames> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !uiFrames) {
return null;
}
Expand All @@ -690,11 +690,17 @@ function Flamegraph(): ReactElement {

return newView;
},
[flamegraphView, flamegraphCanvas, flamegraph, uiFrames, profileOffsetFromTransaction]
);
deps: [
flamegraphView,
flamegraphCanvas,
flamegraph,
uiFrames,
profileOffsetFromTransaction,
],
});

const batteryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>(
_previousView => {
const batteryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !batteryChart || !batteryChartCanvas) {
return null;
}
Expand Down Expand Up @@ -727,18 +733,18 @@ function Flamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
batteryChart,
uiFrames.minFrameDuration,
batteryChartCanvas,
profileOffsetFromTransaction,
]
);
],
});

const cpuChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>(
_previousView => {
const cpuChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !CPUChart || !cpuChartCanvas) {
return null;
}
Expand Down Expand Up @@ -771,18 +777,18 @@ function Flamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
CPUChart,
uiFrames.minFrameDuration,
cpuChartCanvas,
profileOffsetFromTransaction,
]
);
],
});

const memoryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>(
_previousView => {
const memoryChartView = useMemoWithPrevious<CanvasView<FlamegraphChartModel> | null>({
factory: _previousView => {
if (!flamegraphView || !flamegraphCanvas || !memoryChart || !memoryChartCanvas) {
return null;
}
Expand Down Expand Up @@ -815,18 +821,18 @@ function Flamegraph(): ReactElement {

return newView;
},
[
deps: [
flamegraphView,
flamegraphCanvas,
memoryChart,
uiFrames.minFrameDuration,
memoryChartCanvas,
profileOffsetFromTransaction,
]
);
],
});

const spansView = useMemoWithPrevious<CanvasView<SpanChart> | null>(
_previousView => {
const spansView = useMemoWithPrevious<CanvasView<SpanChart> | null>({
factory: _previousView => {
if (!spansCanvas || !spanChart || !flamegraphView) {
return null;
}
Expand All @@ -850,8 +856,8 @@ function Flamegraph(): ReactElement {

return newView;
},
[spanChart, spansCanvas, flamegraphView, flamegraphTheme.SIZES]
);
deps: [spanChart, spansCanvas, flamegraphView, flamegraphTheme.SIZES],
});

// We want to make sure that the views have the same min zoom levels so that
// if you wheel zoom on one, the other one will also zoom to the same level of detail.
Expand Down
8 changes: 4 additions & 4 deletions static/app/components/tours/components.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,16 @@ export function TourContextProvider<T extends TourEnumType>({
}),
[onStartTour, onEndTour, onStepChange, requireAllStepsRegistered]
);
const tourContextValue = useTourReducer<T>(
{
const tourContextValue = useTourReducer<T>({
initialState: {
isCompleted,
isRegistered: false,
orderedStepIds,
currentStepId: null,
tourKey,
},
options
);
options,
});
const {endTour, previousStep, nextStep, currentStepId} = tourContextValue;
const isTourActive = currentStepId !== null;

Expand Down
8 changes: 6 additions & 2 deletions static/app/components/tours/tourContext.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ describe('useTourReducer', () => {
orderedStepIds: ORDERED_TEST_TOUR,
};
function registerAllSteps() {
const {result} = renderHook(() => useTourReducer<TestTour>(initialState, {}));
const {result} = renderHook(() =>
useTourReducer<TestTour>({initialState, options: {}})
);
const {handleStepRegistration} = result.current;
act(() => {
ORDERED_TEST_TOUR.forEach(stepId => handleStepRegistration({id: stepId}));
Expand All @@ -22,7 +24,9 @@ describe('useTourReducer', () => {
}

it('handles step registration correctly', () => {
const {result} = renderHook(() => useTourReducer<TestTour>(initialState, {}));
const {result} = renderHook(() =>
useTourReducer<TestTour>({initialState, options: {}})
);
const {handleStepRegistration} = result.current;
let unregister = () => {};
// Should be false before any steps are registered
Expand Down
11 changes: 7 additions & 4 deletions static/app/components/tours/tourContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,13 @@ function tourReducer<T extends TourEnumType>(
}
}

export function useTourReducer<T extends TourEnumType>(
initialState: TourState<T>,
options: TourOptions<T>
): TourContextType<T> {
export function useTourReducer<T extends TourEnumType>({
initialState,
options,
}: {
initialState: TourState<T>;
options: TourOptions<T>;
}): TourContextType<T> {
const {orderedStepIds} = initialState;

const [state, dispatch] = useReducer(tourReducer<T>, initialState);
Expand Down
24 changes: 12 additions & 12 deletions static/app/utils/demoMode/demoTours.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,20 +152,20 @@ export function DemoToursProvider({children}: {children: React.ReactNode}) {
[handleEndTour, handleStepChange]
);

const issuesTour = useTourReducer<DemoTourStep>(
tourState[DemoTour.ISSUES],
getTourOptions(DemoTour.ISSUES)
);
const issuesTour = useTourReducer<DemoTourStep>({
initialState: tourState[DemoTour.ISSUES],
options: getTourOptions(DemoTour.ISSUES),
});

const releasesTour = useTourReducer<DemoTourStep>(
tourState[DemoTour.RELEASES],
getTourOptions(DemoTour.RELEASES)
);
const releasesTour = useTourReducer<DemoTourStep>({
initialState: tourState[DemoTour.RELEASES],
options: getTourOptions(DemoTour.RELEASES),
});

const performanceTour = useTourReducer<DemoTourStep>(
tourState[DemoTour.PERFORMANCE],
getTourOptions(DemoTour.PERFORMANCE)
);
const performanceTour = useTourReducer<DemoTourStep>({
initialState: tourState[DemoTour.PERFORMANCE],
options: getTourOptions(DemoTour.PERFORMANCE),
});

const tours = useMemo(
() => ({
Expand Down
8 changes: 4 additions & 4 deletions static/app/utils/profiling/flamegraph/flamegraphStateProvider/flamegraphContextProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ function getDefaultState(initialState?: DeepPartial<FlamegraphState>): Flamegrap
export function FlamegraphStateProvider(
props: FlamegraphStateProviderProps
): React.ReactElement {
const [state, dispatch, {nextState, previousState}] = useUndoableReducer(
flamegraphStateReducer,
getDefaultState(props.initialState)
);
const [state, dispatch, {nextState, previousState}] = useUndoableReducer({
reducer: flamegraphStateReducer,
initialState: getDefaultState(props.initialState),
});

const flamegraphContextValue: FlamegraphStateValue = useMemo(() => {
return [state, {nextState, previousState}];
Expand Down
Loading
Loading