Skip to content

Commit

Permalink
refactor(core): Update push/replace navigation to not trigger popstate
Browse files Browse the repository at this point in the history
This matches the spec, though there is a bug in chrome that does trigger these events.
  • Loading branch information
atscott committed Feb 21, 2025
1 parent 64631d2 commit 9b782cb
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 63 deletions.
30 changes: 2 additions & 28 deletions packages/core/primitives/dom-navigation/testing/fake_navigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ export class FakeNavigation implements Navigation {
// Always false for pushState() or replaceState().
userInitiated: false,
hashChange,
triggeredByHistoryApi: true,
});
}

Expand Down Expand Up @@ -456,7 +455,6 @@ export class FakeNavigation implements Navigation {
destination,
info: options.info,
sameDocument: destination.sameDocument,
triggeredByHistoryApi: options.triggeredByHistoryApi,
result,
});
}
Expand All @@ -479,34 +477,14 @@ export class FakeNavigation implements Navigation {
}
// "If navigationType is "push" or "replace", then run the URL and history update steps given document and event's destination's URL, with serialiedData set to event's classic history API state and historyHandling set to navigationType."
if (navigateEvent.navigationType === 'push' || navigateEvent.navigationType === 'replace') {
// TODO(atscott): The spec doesn't have this branch and only does urlAndHistoryUpdateSteps
// but I cannot find where popstate
if (navigateEvent.triggeredByHistoryApi) {
this.urlAndHistoryUpdateSteps(navigateEvent);
} else {
this.updateDocumentForHistoryStepApplication(navigateEvent);
}
this.urlAndHistoryUpdateSteps(navigateEvent);
} else if (navigateEvent.navigationType === 'reload') {
this.updateNavigationEntriesForSameDocumentNavigation(navigateEvent);
} else if (navigateEvent.navigationType === 'traverse') {
// "If navigationType is "traverse", then this event firing is happening as part of the traversal process, and that process will take care of performing the appropriate session history entry updates."
}
}

/**
* https://html.spec.whatwg.org/multipage/browsing-the-web.html#update-document-for-history-step-application
* @param navigateEvent
*/
private updateDocumentForHistoryStepApplication(navigateEvent: InternalFakeNavigateEvent) {
this.updateNavigationEntriesForSameDocumentNavigation(navigateEvent);
// Happens as part of "updating the document" steps https://whatpr.org/html/10919/browsing-the-web.html#updating-the-document
const popStateEvent = createPopStateEvent({
state: navigateEvent.destination.getHistoryState(),
});
this.window.dispatchEvent(popStateEvent);
// TODO(atscott): If oldURL's fragment is not equal to entry's URL's fragment, then queue a global task to fire an event named hashchange
}

/**
* Implementation for a push or replace navigation.
* https://whatpr.org/html/10919/browsing-the-web.html#url-and-history-update-steps
Expand All @@ -533,6 +511,7 @@ export class FakeNavigation implements Navigation {
state: navigateEvent.destination.getHistoryState(),
});
this.window.dispatchEvent(popStateEvent);
// TODO(atscott): If oldURL's fragment is not equal to entry's URL's fragment, then queue a global task to fire an event named hashchange
}

/** https://whatpr.org/html/10919/nav-history-apis.html#update-the-navigation-api-entries-for-a-same-document-navigation */
Expand Down Expand Up @@ -770,7 +749,6 @@ export interface FakeNavigateEvent extends ExperimentalNavigateEvent {

interface InternalFakeNavigateEvent extends FakeNavigateEvent {
readonly sameDocument: boolean;
readonly triggeredByHistoryApi?: boolean;
readonly commitOption: 'after-transition' | 'immediate';
readonly result: InternalNavigationResult;
interceptionState: 'none' | 'intercepted' | 'committed' | 'scrolled' | 'finished';
Expand All @@ -797,7 +775,6 @@ function dispatchNavigateEvent({
destination,
info,
sameDocument,
triggeredByHistoryApi,
result,
}: {
cancelable: boolean;
Expand All @@ -809,7 +786,6 @@ function dispatchNavigateEvent({
destination: FakeNavigationDestination;
info: unknown;
sameDocument: boolean;
triggeredByHistoryApi?: boolean;
result: InternalNavigationResult;
}) {
const {navigation} = result;
Expand All @@ -831,7 +807,6 @@ function dispatchNavigateEvent({
event.result = result;

event.sameDocument = sameDocument;
event.triggeredByHistoryApi = triggeredByHistoryApi;
event.commitOption = 'immediate';

let handlersFinished = [Promise.resolve()];
Expand Down Expand Up @@ -1164,5 +1139,4 @@ interface InternalNavigateOptions {
userInitiated: boolean;
hashChange: boolean;
info?: unknown;
triggeredByHistoryApi?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,15 +195,13 @@ describe('navigation', () => {
}),
);
expect(currentEntryChangeEvent.from.getState()).toBe(initialEntry.getState());
expect(locals.popStateEvents.length).toBe(1);
const popStateEvent = locals.popStateEvents[0];
expect(popStateEvent.state).toBeNull();
expect(locals.popStateEvents.length).toBe(0);
const finishedEntry = await finished;
expect(committedEntry).toBe(finishedEntry);
expect(locals.navigation.currentEntry).toBe(finishedEntry);
expect(locals.navigateEvents.length).toBe(1);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
});

it('push URL relative', async () => {
Expand Down Expand Up @@ -270,15 +268,13 @@ describe('navigation', () => {
}),
);
expect(currentEntryChangeEvent.from.getState()).toBe(initialEntry.getState());
expect(locals.popStateEvents.length).toBe(1);
const popStateEvent = locals.popStateEvents[0];
expect(popStateEvent.state).toBeNull();
expect(locals.popStateEvents.length).toBe(0);
const finishedEntry = await finished;
expect(committedEntry).toBe(finishedEntry);
expect(locals.navigation.currentEntry).toBe(finishedEntry);
expect(locals.navigateEvents.length).toBe(1);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
});

it('push URL with state', async () => {
Expand All @@ -293,7 +289,7 @@ describe('navigation', () => {
const committedEntry = await committed;
expect(committedEntry.getState()).toEqual(state);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
const finishedEntry = await finished;
expect(committedEntry).toBe(finishedEntry);
});
Expand All @@ -311,7 +307,7 @@ describe('navigation', () => {
const committedEntry = await committed;
expect(committedEntry.getState()).toEqual(state);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
const finishedEntry = await finished;
expect(committedEntry).toBe(finishedEntry);
});
Expand All @@ -325,7 +321,7 @@ describe('navigation', () => {
const committedEntry = await committed;
expect(committedEntry.url).toBe('https://test.com/#test');
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
const finishedEntry = await finished;
expect(committedEntry).toBe(finishedEntry);
});
Expand All @@ -341,7 +337,7 @@ describe('navigation', () => {
const committedEntry = await committed;
expect(committedEntry.url).toBe('https://test.com/#test');
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
const finishedEntry = await finished;
expect(committedEntry).toBe(finishedEntry);
});
Expand All @@ -355,7 +351,7 @@ describe('navigation', () => {
expect(navigateEvent.info).toBe(info);
await committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await finished;
});

Expand All @@ -371,7 +367,7 @@ describe('navigation', () => {
expect(navigateEvent.info).toBe(info);
await committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await finished;
});

Expand All @@ -397,7 +393,7 @@ describe('navigation', () => {
);
expect(committedEntry.getState()).toBeUndefined();
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
handlerFinishedResolve(undefined);
await expectAsync(finished).toBeResolvedTo(committedEntry);
Expand Down Expand Up @@ -427,7 +423,7 @@ describe('navigation', () => {
);
expect(committedEntry.getState()).toBeUndefined();
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
handlerFinishedResolve(undefined);
await expectAsync(finished).toBeResolvedTo(committedEntry);
Expand Down Expand Up @@ -459,7 +455,7 @@ describe('navigation', () => {
);
expect(committedEntry.getState()).toBeUndefined();
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
handlerFinishedResolve(undefined);
await expectAsync(finished).toBeResolvedTo(committedEntry);
Expand Down Expand Up @@ -491,7 +487,7 @@ describe('navigation', () => {
);
expect(committedEntry.getState()).toBeUndefined();
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBeResolvedTo(committedEntry);
});

Expand Down Expand Up @@ -573,7 +569,7 @@ describe('navigation', () => {
);
expect(committedEntry.getState()).toBeUndefined();
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBeResolvedTo(committedEntry);
});

Expand All @@ -598,15 +594,15 @@ describe('navigation', () => {
const navigateEvent = locals.navigateEvents[0];
await committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
locals.pendingInterceptOptions.push({});
const interruptResult = locals.navigation.navigate('/interrupt');
await expectAsync(finished).toBeRejectedWithError(DOMException);
expect(navigateEvent.signal.aborted).toBeTrue();
await interruptResult.committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(0);
await interruptResult.finished;
});

Expand All @@ -622,15 +618,15 @@ describe('navigation', () => {
const navigateEvent = locals.navigateEvents[0];
await committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
locals.pendingInterceptOptions.push({});
const interruptResult = locals.navigation.navigate('/interrupt');
await expectAsync(finished).toBeRejectedWithError(DOMException);
expect(navigateEvent.signal.aborted).toBeTrue();
await interruptResult.committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(0);
await interruptResult.finished;
});

Expand All @@ -648,7 +644,7 @@ describe('navigation', () => {
const navigateEvent = locals.navigateEvents[0];
await committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
const error = new Error('rejected');
handlerFinishedReject(error);
Expand All @@ -672,7 +668,7 @@ describe('navigation', () => {
const navigateEvent = locals.navigateEvents[0];
await committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await expectAsync(finished).toBePending();
const error = new Error('rejected');
handlerFinishedReject(error);
Expand Down Expand Up @@ -900,7 +896,7 @@ describe('navigation', () => {
expect(navigateEvent.signal.aborted).toBeTrue();
await interruptResult.committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(1);
await interruptResult.finished;
});

Expand Down Expand Up @@ -1192,7 +1188,7 @@ describe('navigation', () => {
}),
);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
const navigateResultFinishedEntry = await navigateResult.finished;
expect(navigateResultFinishedEntry).toBe(navigateResultCommittedEntry);
expect(locals.navigation.currentEntry).toBe(navigateResultCommittedEntry);
Expand All @@ -1211,7 +1207,7 @@ describe('navigation', () => {
}),
);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(1);
const firstTraverseFinishedEntry = await firstTraverseResult.finished;
expect(firstTraverseFinishedEntry).toBe(firstPageEntry);
expect(locals.navigation.currentEntry).toBe(firstPageEntry);
Expand All @@ -1230,7 +1226,7 @@ describe('navigation', () => {
}),
);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(3);
expect(locals.popStateEvents.length).toBe(3);
expect(locals.popStateEvents.length).toBe(2);
const secondTraverseFinishedEntry = await secondTraverseResult.finished;
expect(secondTraverseFinishedEntry).toBe(secondPageEntry);
expect(locals.navigation.currentEntry).toBe(secondPageEntry);
Expand Down Expand Up @@ -1458,7 +1454,7 @@ describe('navigation', () => {
expect(navigateEvent.signal.aborted).toBeTrue();
await interruptResult.committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await interruptResult.finished;
});

Expand All @@ -1477,7 +1473,7 @@ describe('navigation', () => {
expect(navigateEvent.signal.aborted).toBeTrue();
await interruptResult.committed;
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
await interruptResult.finished;
});

Expand Down Expand Up @@ -1705,7 +1701,7 @@ describe('navigation', () => {
await interruptResult.committed;
expect(navigateEvent.signal.aborted).toBeTrue();
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(1);
await interruptResult.finished;
});

Expand Down Expand Up @@ -1836,17 +1832,17 @@ describe('navigation', () => {
const interruptEntry = await interruptResult.finished;
expect(locals.navigation.currentEntry).toBe(interruptEntry);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(1);
expect(locals.popStateEvents.length).toBe(0);
const firstNavigateEvent = await locals.nextNavigateEvent();
expect(firstNavigateEvent.destination.key).toBe(secondPageEntry.key);
expect(locals.navigation.currentEntry).toBe(secondPageEntry);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(2);
expect(locals.popStateEvents.length).toBe(1);
const secondNavigateEvent = await locals.nextNavigateEvent();
expect(secondNavigateEvent.destination.key).toBe(thirdPageEntry.key);
expect(locals.navigation.currentEntry).toBe(thirdPageEntry);
expect(locals.navigationCurrentEntryChangeEvents.length).toBe(3);
expect(locals.popStateEvents.length).toBe(3);
expect(locals.popStateEvents.length).toBe(2);
});
});
});
Expand Down

0 comments on commit 9b782cb

Please sign in to comment.