From 40e64859e7fa54cf0085aff8cc56830617ced1f9 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Thu, 17 Nov 2022 15:36:58 -0800 Subject: [PATCH 1/2] fix(common): Fix MockPlatformLocation events and missing onPopState implementation The MockPlatformLocation forward, back, and historyGo methods should trigger a popstate event. Additionally, these events should just be synchronous since that's what the majority of the major browsers do. Lastly, onPopState should be implemented the same way as onHashChange. --- .../testing/src/mock_platform_location.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/common/testing/src/mock_platform_location.ts b/packages/common/testing/src/mock_platform_location.ts index 02c5b99003f56..49bbc9f4f3ef5 100644 --- a/packages/common/testing/src/mock_platform_location.ts +++ b/packages/common/testing/src/mock_platform_location.ts @@ -105,6 +105,7 @@ export const MOCK_PLATFORM_LOCATION_CONFIG = export class MockPlatformLocation implements PlatformLocation { private baseHref: string = ''; private hashUpdate = new Subject(); + private popStateSubject = new Subject(); private urlChangeIndex: number = 0; private urlChanges: { hostname: string, @@ -155,9 +156,8 @@ export class MockPlatformLocation implements PlatformLocation { } onPopState(fn: LocationChangeListener): VoidFunction { - // No-op: a state stack is not implemented, so - // no events will ever come. - return () => {}; + const subscription = this.popStateSubject.subscribe(fn); + return () => subscription.unsubscribe(); } onHashChange(fn: LocationChangeListener): VoidFunction { @@ -231,14 +231,18 @@ export class MockPlatformLocation implements PlatformLocation { } private scheduleHashUpdate(oldHash: string, oldUrl: string) { + // Browsers are inconsistent in when they fire events and perform the state updates + // The most easiest thing to do in our mock is synchronous and that happens to match + // Firefox and Chrome, at least somewhat closely + // + // https://github.com/WICG/navigation-api#watching-for-navigations + // https://docs.google.com/document/d/1Pdve-DJ1JCGilj9Yqf5HxRJyBKSel5owgOvUJqTauwU/edit#heading=h.3ye4v71wsz94 + this.popStateSubject.next( + {type: 'popstate', state: this.getState(), oldUrl, newUrl: this.url} as + LocationChangeEvent); if (oldHash !== this.hash) { - scheduleMicroTask( - () => this.hashUpdate.next( - {type: 'hashchange', state: null, oldUrl, newUrl: this.url} as LocationChangeEvent)); + this.hashUpdate.next( + {type: 'hashchange', state: null, oldUrl, newUrl: this.url} as LocationChangeEvent); } } } - -export function scheduleMicroTask(cb: () => any) { - Promise.resolve().then(cb); -} From 91990edc247d7c0767bd538095163a50ae09b5b4 Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Sun, 20 Nov 2022 07:25:33 -0800 Subject: [PATCH 2/2] fixup! fix(common): Fix MockPlatformLocation events and missing onPopState implementation --- .../testing/src/mock_platform_location.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/common/testing/src/mock_platform_location.ts b/packages/common/testing/src/mock_platform_location.ts index 49bbc9f4f3ef5..5ab18e1ce7610 100644 --- a/packages/common/testing/src/mock_platform_location.ts +++ b/packages/common/testing/src/mock_platform_location.ts @@ -204,7 +204,7 @@ export class MockPlatformLocation implements PlatformLocation { if (this.urlChangeIndex < this.urlChanges.length) { this.urlChangeIndex++; } - this.scheduleHashUpdate(oldHash, oldUrl); + this.emitEvents(oldHash, oldUrl); } back(): void { @@ -213,7 +213,7 @@ export class MockPlatformLocation implements PlatformLocation { if (this.urlChangeIndex > 0) { this.urlChangeIndex--; } - this.scheduleHashUpdate(oldHash, oldUrl); + this.emitEvents(oldHash, oldUrl); } historyGo(relativePosition: number = 0): void { @@ -223,20 +223,24 @@ export class MockPlatformLocation implements PlatformLocation { if (nextPageIndex >= 0 && nextPageIndex < this.urlChanges.length) { this.urlChangeIndex = nextPageIndex; } - this.scheduleHashUpdate(oldHash, oldUrl); + this.emitEvents(oldHash, oldUrl); } getState(): unknown { return this.state; } - private scheduleHashUpdate(oldHash: string, oldUrl: string) { - // Browsers are inconsistent in when they fire events and perform the state updates - // The most easiest thing to do in our mock is synchronous and that happens to match - // Firefox and Chrome, at least somewhat closely - // - // https://github.com/WICG/navigation-api#watching-for-navigations - // https://docs.google.com/document/d/1Pdve-DJ1JCGilj9Yqf5HxRJyBKSel5owgOvUJqTauwU/edit#heading=h.3ye4v71wsz94 + /** + * Browsers are inconsistent in when they fire events and perform the state updates + * The most easiest thing to do in our mock is synchronous and that happens to match + * Firefox and Chrome, at least somewhat closely + * + * https://github.com/WICG/navigation-api#watching-for-navigations + * https://docs.google.com/document/d/1Pdve-DJ1JCGilj9Yqf5HxRJyBKSel5owgOvUJqTauwU/edit#heading=h.3ye4v71wsz94 + * popstate is always sent before hashchange: + * https://developer.mozilla.org/en-US/docs/Web/API/Window/popstate_event#when_popstate_is_sent + */ + private emitEvents(oldHash: string, oldUrl: string) { this.popStateSubject.next( {type: 'popstate', state: this.getState(), oldUrl, newUrl: this.url} as LocationChangeEvent);