Skip to content

fix: Ensure LocationFull validates state data before relaying it to history API #55

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

Merged
merged 3 commits into from
Sep 7, 2025
Merged

fix: Ensure LocationFull validates state data before relaying it to history API #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46d5714
fix: Ensure LocationFull validates state data before relaying it to h…
webJose Sep 7, 2025
711d94c
chore(tests): Rename LocationFull's test file and do proper cleanup
webJose Sep 7, 2025
7df9b33
tests: Add unit tests for unsubscription function for location.on()
webJose Sep 7, 2025
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
124 changes: 103 additions & 21 deletions src/lib/core/LocationFull.svelte.test.ts → src/lib/core/LocationFull.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
import { LocationFull } from "./LocationFull.js";
import type { State, Location } from "$lib/types.js";
import { flushSync } from "svelte";
import { joinPaths } from "./RouterEngine.svelte.js";

describe("LocationFull", () => {
Expand Down Expand Up @@ -58,14 +57,46 @@ describe("LocationFull", () => {
test("Should register the provided callback for the 'beforeNavigate' event.", () => {
// Arrange.
const callback = vi.fn();
location.on('beforeNavigate', callback);
const unSub = location.on('beforeNavigate', callback);

// Act.
globalThis.window.history.pushState(null, '', 'http://example.com/other');
flushSync();

// Assert.
expect(callback).toHaveBeenCalledOnce();

// Cleanup.
unSub();
});
test("Should unregister the provided callback when the returned function is called.", () => {
// Arrange.
const callback = vi.fn();
const unSub = location.on('beforeNavigate', callback);

// Act.
unSub();

// Assert.
globalThis.window.history.pushState(null, '', 'http://example.com/other');
expect(callback).not.toHaveBeenCalled();
});
test("Should not affect other handlers when unregistering one of the event handlers.", () => {
// Arrange.
const callback1 = vi.fn();
const callback2 = vi.fn();
const unSub1 = location.on('beforeNavigate', callback1);
const unSub2 = location.on('beforeNavigate', callback2);

// Act.
unSub1();

// Assert.
globalThis.window.history.pushState(null, '', 'http://example.com/other');
expect(callback1).not.toHaveBeenCalled();
expect(callback2).toHaveBeenCalledOnce();

// Cleanup.
unSub2();
});
test.each([
{
Expand All @@ -79,13 +110,12 @@ describe("LocationFull", () => {
])("Should provide the URL, state and method $method via the event object of 'beforeNavigate'.", ({ method, stateFn }) => {
// Arrange.
const callback = vi.fn();
const state = { test: 'value' };
location.on('beforeNavigate', callback);
const state = { path: { test: 'value' }, hash: {} };
const unSub = location.on('beforeNavigate', callback);

// Act.
// @ts-expect-error stateFn cannot enumerate history.
globalThis.window.history[stateFn](state, '', 'http://example.com/other');
flushSync();

// Assert.
expect(callback).toHaveBeenCalledWith({
Expand All @@ -96,16 +126,18 @@ describe("LocationFull", () => {
cancelReason: undefined,
cancel: expect.any(Function)
});

// Cleanup.
unSub();
});
test("Should set wasCancelled to true and cancelReason to the provided reason when the event is cancelled to subsequent callbacks.", () => {
// Arrange.
const callback = vi.fn();
location.on('beforeNavigate', (event) => event.cancel('test'));
location.on('beforeNavigate', callback);
const unSub1 = location.on('beforeNavigate', (event) => event.cancel('test'));
const unSub2 = location.on('beforeNavigate', callback);

// Act.
globalThis.window.history.pushState(null, '', 'http://example.com/other');
flushSync();

// Assert.
expect(callback).toHaveBeenCalledWith({
Expand All @@ -116,17 +148,20 @@ describe("LocationFull", () => {
cancelReason: 'test',
cancel: expect.any(Function)
});

// Cleanup.
unSub1();
unSub2();
});
test("Should ignore cancellation reasons from callbacks if the event has already been cancelled.", () => {
// Arrange.
const callback = vi.fn();
location.on('beforeNavigate', (event) => event.cancel('test'));
location.on('beforeNavigate', (event) => event.cancel('ignored'));
location.on('beforeNavigate', callback);
const unSub1 = location.on('beforeNavigate', (event) => event.cancel('test'));
const unSub2 = location.on('beforeNavigate', (event) => event.cancel('ignored'));
const unSub3 = location.on('beforeNavigate', callback);

// Act.
globalThis.window.history.pushState(null, '', 'http://example.com/other');
flushSync();

// Assert.
expect(callback).toHaveBeenCalledWith({
Expand All @@ -137,34 +172,66 @@ describe("LocationFull", () => {
cancelReason: 'test',
cancel: expect.any(Function)
});

// Cleanup.
unSub1();
unSub2();
unSub3();
});
test.each([
'pushState' as const,
'replaceState' as const,
])("Should ultimately push the state data via the %s method set by beforeNavigate handlers in event.state.", (stateFn) => {
// Arrange.
const state = { path: { test: 'value' }, hash: {} };
const callback = vi.fn((event) => {
event.state = state;
});
const unSub = location.on('beforeNavigate', callback);

// Act.
globalThis.window.history[stateFn](null, '', 'http://example.com/other');

// Assert.
expect(callback).toHaveBeenCalledOnce();
expect(globalThis.window.history.state).deep.equal(state);

// Cleanup.
unSub();
});
test("Should register the provided callback for the 'navigationCancelled' event.", () => {
// Arrange.
const callback = vi.fn();
location.on('beforeNavigate', (event) => event.cancel());
location.on('navigationCancelled', callback);
const unSub1 = location.on('beforeNavigate', (event) => event.cancel());
const unSub2 = location.on('navigationCancelled', callback);

// Act.
globalThis.window.history.pushState(null, '', 'http://example.com/other');
flushSync();

// Assert.
expect(callback).toHaveBeenCalledOnce();

// Cleanup.
unSub1();
unSub2();
});
test("Should transfer the cause of cancellation and the state to the 'navigationCancelled' event.", () => {
// Arrange.
const callback = vi.fn();
const reason = 'test';
const state = { test: 'value' };
location.on('beforeNavigate', (event) => event.cancel(reason));
location.on('navigationCancelled', callback);
const unSub1 = location.on('beforeNavigate', (event) => event.cancel(reason));
const unSub2 = location.on('navigationCancelled', callback);

// Act.
globalThis.window.history.pushState(state, '', 'http://example.com/other');
flushSync();

// Assert.
expect(callback).toHaveBeenCalledWith({ url: 'http://example.com/other', cause: 'test', method: 'push', state });

// Cleanup.
unSub1();
unSub2();
});
});
describe('url', () => {
Expand All @@ -177,7 +244,6 @@ describe("LocationFull", () => {

// Act.
globalThis.window.history[fn](null, '', newUrl);
flushSync();

// Assert.
expect(location.url.href).toBe(newUrl);
Expand All @@ -193,12 +259,28 @@ describe("LocationFull", () => {

// Act.
globalThis.window.history[fn](state, '', 'http://example.com/new');
flushSync();

// Assert.
expect(location.getState(false)).toEqual(state.path);
expect(location.getState(true)).toEqual(state.hash.single);
expect(location.getState('p1')).toEqual(state.hash.p1);
});
});
describe('Navigation Interception', () => {
test.each([
'pushState' as const,
'replaceState' as const,
])("Should preserve the previous valid state whenever %s is called with non-conformant state.", (stateFn) => {
// Arrange.
const validState = { path: { test: 'value' }, hash: {} };
globalThis.window.history[stateFn](validState, '', 'http://example.com/');
const state = { test: 'value' };

// Act.
globalThis.window.history[stateFn](state, '', 'http://example.com/other');

// Assert.
expect(globalThis.window.history.state).deep.equals(validState);
});
});
});
7 changes: 6 additions & 1 deletion src/lib/core/LocationFull.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { BeforeNavigateEvent, Events, NavigationCancelledEvent, NavigationEvent } from "$lib/types.js";
import { isConformantState } from "./isConformantState.js";
import { LocationLite } from "./LocationLite.svelte.js";
import { LocationState } from "./LocationState.svelte.js";

Expand Down Expand Up @@ -51,12 +52,16 @@ export class LocationFull extends LocationLite {
for (let sub of Object.values(this.#eventSubs.navigationCancelled)) {
sub({
url,
state,
state: event.state,
method,
cause: event.cancelReason,
});
}
} else {
if (!isConformantState(event.state)) {
console.warn("Warning: Non-conformant state object passed to history." + method + "State. Previous state will prevail.");
event.state = this.#innerState.state;
}
const navFn = method === 'push' ? this.#originalPushState : this.#originalReplaceState;
navFn(event.state, '', url);
this.url.href = globalThis.window?.location.href;
Expand Down