Skip to content

Commit b451387

Browse files
committed
Make it so the replaceState idea actually works.
1 parent 977c2c7 commit b451387

2 files changed

Lines changed: 32 additions & 12 deletions

File tree

src/actions/profile-view.ts

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
getHiddenLocalTracks,
3535
getInvertCallstack,
3636
getHash,
37+
getUrlState,
3738
} from 'firefox-profiler/selectors/url-state';
3839
import {
3940
assertExhaustiveCheck,
@@ -81,7 +82,7 @@ import {
8182
funcHasRecursiveCall,
8283
} from '../profile-logic/transforms';
8384
import { changeStoredProfileNameInDb } from 'firefox-profiler/app-logic/uploaded-profiles-db';
84-
import { withHistoryReplaceStateSync } from 'firefox-profiler/app-logic/url-handling';
85+
import { replaceHistoryWithUrlState } from 'firefox-profiler/app-logic/url-handling';
8586
import type { TabSlug } from '../app-logic/tabs-handling';
8687
import type { CallNodeInfo } from '../profile-logic/call-node-info';
8788
import type { SingleColumnSortState } from '../components/shared/TreeView';
@@ -165,24 +166,26 @@ export function changeUpperWingSelectedCallNode(
165166
/**
166167
* Select a function for a given thread in the function list.
167168
*
168-
* Uses replaceState rather than pushState so that holding e.g. the down arrow
169-
* key in the function list doesn't get rate-limited by the browser and doesn't
170-
* flood the back/forward history.
169+
* Replaces the current history entry rather than pushing a new one, so that
170+
* holding e.g. the down arrow key in the function list doesn't get rate-limited
171+
* by the browser and doesn't flood the back/forward history.
171172
*/
172173
export function changeSelectedFunctionIndex(
173174
threadsKey: ThreadsKey,
174175
selectedFunctionIndex: IndexIntoFuncTable | null,
175176
context: SelectionContext = { source: 'auto' }
176177
): ThunkAction<void> {
177-
return (dispatch) => {
178-
withHistoryReplaceStateSync(() => {
179-
dispatch({
180-
type: 'CHANGE_SELECTED_FUNCTION',
181-
selectedFunctionIndex,
182-
threadsKey,
183-
context,
184-
});
178+
return (dispatch, getState) => {
179+
dispatch({
180+
type: 'CHANGE_SELECTED_FUNCTION',
181+
selectedFunctionIndex,
182+
threadsKey,
183+
context,
185184
});
185+
// Update window.history synchronously instead of waiting for the
186+
// UrlManager's componentDidUpdate, which is deferred by React's render
187+
// scheduling and would otherwise pushState a new entry.
188+
replaceHistoryWithUrlState(getUrlState(getState()));
186189
};
187190
}
188191

src/app-logic/url-handling.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,23 @@ export function getIsHistoryReplaceState(): boolean {
122122
return _isReplaceState;
123123
}
124124

125+
/**
126+
* Synchronously replace the current history entry to match the given UrlState.
127+
*
128+
* Use this from action thunks for high-frequency actions (e.g. arrow-key
129+
* navigation) where deferring the URL update to UrlManager.componentDidUpdate
130+
* would result in unwanted pushState calls and history-flooding. By updating
131+
* window.history synchronously here, the URL already matches the new state by
132+
* the time UrlManager's componentDidUpdate runs, so it becomes a no-op.
133+
*/
134+
export function replaceHistoryWithUrlState(urlState: UrlState): void {
135+
window.history.replaceState(
136+
urlState,
137+
document.title,
138+
urlFromState(urlState)
139+
);
140+
}
141+
125142
function getPathParts(urlState: UrlState): string[] {
126143
const { dataSource } = urlState;
127144
switch (dataSource) {

0 commit comments

Comments
 (0)