From efe1ea0a3f0172c9ad4c72b7bf01daddd5150650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ramirez=20Vargas=2C=20Jos=C3=A9=20Pablo?= Date: Sun, 23 Nov 2025 15:34:35 -0600 Subject: [PATCH 1/2] feat: Add runtime check that validates the defaultHash/hashMode combo --- src/lib/kernel/options.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/kernel/options.ts b/src/lib/kernel/options.ts index 3b5aa7e..0ef4b3d 100644 --- a/src/lib/kernel/options.ts +++ b/src/lib/kernel/options.ts @@ -28,6 +28,9 @@ export function setRoutingOptions(options?: Partial): vo routingOptions.disallowPathRouting = options?.disallowPathRouting ?? routingOptions.disallowPathRouting; routingOptions.disallowHashRouting = options?.disallowHashRouting ?? routingOptions.disallowHashRouting; routingOptions.disallowMultiHashRouting = options?.disallowMultiHashRouting ?? routingOptions.disallowMultiHashRouting; + if (routingOptions.hashMode === 'single' && typeof routingOptions.defaultHash === 'string') { + throw new Error("Using a named hash path as the default path can only be done when 'hashMode' is set to 'multi'."); + } } /** From 658aee38d7bff5a7e5753e5f895ec38ca27204c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ramirez=20Vargas=2C=20Jos=C3=A9=20Pablo?= Date: Sun, 23 Nov 2025 16:04:27 -0600 Subject: [PATCH 2/2] feat: Add runtime check for the multi/true hashMode/defaultHash combo --- src/lib/kernel/initCore.test.ts | 68 ++++++++-------- src/lib/kernel/options.test.ts | 132 ++++++++++++++++++++++++++++++-- src/lib/kernel/options.ts | 3 + 3 files changed, 162 insertions(+), 41 deletions(-) diff --git a/src/lib/kernel/initCore.test.ts b/src/lib/kernel/initCore.test.ts index d0ca684..a963c4b 100644 --- a/src/lib/kernel/initCore.test.ts +++ b/src/lib/kernel/initCore.test.ts @@ -4,7 +4,7 @@ import { initCore } from './initCore.js'; import { SvelteURL } from "svelte/reactivity"; import { location } from "./Location.js"; import { defaultTraceOptions, traceOptions } from "./trace.svelte.js"; -import { defaultRoutingOptions, routingOptions } from "./options.js"; +import { defaultRoutingOptions, resetRoutingOptions, routingOptions } from "./options.js"; import { logger } from "./Logger.js"; const initialUrl = 'http://example.com/'; @@ -19,6 +19,7 @@ const locationMock: Location = { on: vi.fn(), go: vi.fn(), navigate: vi.fn(), + get path() { return this.url.pathname; }, }; describe('initCore', () => { @@ -26,6 +27,7 @@ describe('initCore', () => { afterEach(() => { vi.resetAllMocks(); cleanup?.(); + resetRoutingOptions(); }); test("Should initialize with all the expected default values.", () => { // Act. @@ -53,20 +55,10 @@ describe('initCore', () => { error: () => { } }; - // Capture initial state - const initialLoggerIsOffLogger = logger !== globalThis.console; - const initialRoutingOptions = { - hashMode: routingOptions.hashMode, - defaultHash: routingOptions.defaultHash - }; - const initialTraceOptions = { - routerHierarchy: traceOptions.routerHierarchy - }; - - // Act - Initialize with custom options + // Act - Initialize with custom options (use valid combo) cleanup = initCore(locationMock, { hashMode: 'multi', - defaultHash: true, + defaultHash: 'customHash', logger: customLogger, trace: { routerHierarchy: true @@ -76,7 +68,7 @@ describe('initCore', () => { // Assert - Check that options were applied expect(logger).toBe(customLogger); expect(routingOptions.hashMode).toBe('multi'); - expect(routingOptions.defaultHash).toBe(true); + expect(routingOptions.defaultHash).toBe('customHash'); expect(traceOptions.routerHierarchy).toBe(true); expect(location).toBeDefined(); @@ -84,11 +76,11 @@ describe('initCore', () => { cleanup(); cleanup = undefined; - // Assert - Check that everything was rolled back - expect(logger !== globalThis.console).toBe(initialLoggerIsOffLogger); // Back to offLogger - expect(routingOptions.hashMode).toBe(initialRoutingOptions.hashMode); - expect(routingOptions.defaultHash).toBe(initialRoutingOptions.defaultHash); - expect(traceOptions.routerHierarchy).toBe(initialTraceOptions.routerHierarchy); + // Assert - Check that everything was rolled back to library defaults + expect(logger).not.toBe(customLogger); // Should revert to offLogger + expect(routingOptions.hashMode).toBe(defaultRoutingOptions.hashMode); + expect(routingOptions.defaultHash).toBe(defaultRoutingOptions.defaultHash); + expect(traceOptions.routerHierarchy).toBe(defaultTraceOptions.routerHierarchy); expect(location).toBeNull(); }); test("Should throw an error when called a second time without proper prior cleanup.", () => { @@ -103,53 +95,57 @@ describe('initCore', () => { }); describe('cleanup', () => { test("Should rollback everything to defaults.", async () => { - // Arrange. - const uninitializedLogger = logger; + // Arrange - Initialize with custom options cleanup = initCore(locationMock, { hashMode: 'multi', - defaultHash: true, + defaultHash: 'abc', trace: { routerHierarchy: !defaultTraceOptions.routerHierarchy } }); - // Verify options were applied + // Verify options were applied (no type conversion occurs) expect(routingOptions.hashMode).toBe('multi'); - expect(routingOptions.defaultHash).toBe(true); - expect(logger).not.toBe(uninitializedLogger); + expect(routingOptions.defaultHash).toBe('abc'); + expect(logger).toBe(globalThis.console); // Default logger when none specified expect(traceOptions.routerHierarchy).toBe(!defaultTraceOptions.routerHierarchy); // Act - Cleanup cleanup(); cleanup = undefined; - // Assert - Check that routing options were reset to defaults - expect(routingOptions.hashMode).toBe('single'); - expect(routingOptions.defaultHash).toBe(false); - expect(logger).toBe(uninitializedLogger); + // Assert - Check that all options were reset to library defaults + expect(routingOptions.hashMode).toBe(defaultRoutingOptions.hashMode); + expect(routingOptions.defaultHash).toBe(defaultRoutingOptions.defaultHash); + expect(logger).not.toBe(globalThis.console); // Reverts to offLogger (uninitialized state) expect(traceOptions).toEqual(defaultTraceOptions); }); test("Should handle multiple init/cleanup cycles properly.", async () => { - // Arrange. - const uninitializedLogger = logger; + // Capture initial logger state for comparison + const initialLogger = logger; + + // First cycle cleanup = initCore(locationMock, { logger: { debug: () => { }, log: () => { }, warn: () => { }, error: () => { } } }); - expect(logger).not.toBe(uninitializedLogger); + expect(logger).not.toBe(initialLogger); expect(location).toBeDefined(); + cleanup(); cleanup = undefined; - expect(logger).toBe(uninitializedLogger); + expect(logger).toBe(initialLogger); // Back to initial state expect(location).toBeNull(); + + // Second cycle cleanup = initCore(locationMock, { hashMode: 'multi' }); expect(routingOptions.hashMode).toBe('multi'); expect(location).toBeDefined(); - // Act. + // Act - Final cleanup cleanup(); cleanup = undefined; - // Assert. - expect(routingOptions.hashMode).toBe('single'); + // Assert - Should revert to library defaults + expect(routingOptions.hashMode).toBe(defaultRoutingOptions.hashMode); expect(location).toBeNull(); }); }); diff --git a/src/lib/kernel/options.test.ts b/src/lib/kernel/options.test.ts index 3c3bad4..729fedb 100644 --- a/src/lib/kernel/options.test.ts +++ b/src/lib/kernel/options.test.ts @@ -1,5 +1,5 @@ -import { describe, expect, test } from "vitest"; -import { resetRoutingOptions, routingOptions } from "./options.js"; +import { describe, expect, test, beforeEach } from "vitest"; +import { resetRoutingOptions, routingOptions, setRoutingOptions } from "./options.js"; describe("options", () => { test("Should have correct default value for hashMode option.", () => { @@ -47,10 +47,132 @@ describe("options", () => { expect(typeof routingOptions.defaultHash).toBe('boolean'); }); + describe('setRoutingOptions', () => { + beforeEach(() => { + // Reset to defaults before each test + resetRoutingOptions(); + }); + + test("Should merge options with current values when partial options provided.", () => { + // Arrange - Set initial non-default values + routingOptions.hashMode = 'multi'; + routingOptions.defaultHash = 'customHash'; + + // Act - Set only one option + setRoutingOptions({ disallowPathRouting: true }); + + // Assert - Only specified option changed, others preserved + expect(routingOptions.hashMode).toBe('multi'); + expect(routingOptions.defaultHash).toBe('customHash'); + expect(routingOptions.disallowPathRouting).toBe(true); + expect(routingOptions.disallowHashRouting).toBe(false); + }); + + test("Should set all options when full configuration provided.", () => { + // Arrange & Act + setRoutingOptions({ + hashMode: 'multi', + defaultHash: 'namedHash', + disallowPathRouting: true, + disallowHashRouting: true, + disallowMultiHashRouting: false + }); + + // Assert + expect(routingOptions.hashMode).toBe('multi'); + expect(routingOptions.defaultHash).toBe('namedHash'); + expect(routingOptions.disallowPathRouting).toBe(true); + expect(routingOptions.disallowHashRouting).toBe(true); + expect(routingOptions.disallowMultiHashRouting).toBe(false); + }); + + test("Should do nothing when called with undefined options.", () => { + // Arrange - Set initial values + const original = structuredClone(routingOptions); + + // Act + setRoutingOptions(undefined); + + // Assert - No changes + expect(routingOptions).deep.equal(original); + }); + + test("Should do nothing when called with empty options.", () => { + // Arrange - Set initial values + const original = structuredClone(routingOptions); + + // Act + setRoutingOptions({}); + + // Assert - No changes + expect(routingOptions).deep.equal(original); + }); + + describe('Runtime validation', () => { + test("Should throw error when hashMode is 'single' and defaultHash is a string.", () => { + // Arrange & Act & Assert + expect(() => { + setRoutingOptions({ + hashMode: 'single', + defaultHash: 'namedHash' + }); + }).toThrow("Using a named hash path as the default path can only be done when 'hashMode' is set to 'multi'."); + }); + + test("Should throw error when hashMode is 'multi' and defaultHash is true.", () => { + // Arrange & Act & Assert + expect(() => { + setRoutingOptions({ + hashMode: 'multi', + defaultHash: true + }); + }).toThrow("Using classic hash routing as default can only be done when 'hashMode' is set to 'single'."); + }); + + test("Should throw error when existing hashMode is 'single' and setting defaultHash to string.", () => { + // Arrange + routingOptions.hashMode = 'single'; + + // Act & Assert + expect(() => { + setRoutingOptions({ defaultHash: 'namedHash' }); + }).toThrow("Using a named hash path as the default path can only be done when 'hashMode' is set to 'multi'."); + }); + + test("Should throw error when existing defaultHash is true and setting hashMode to 'multi'.", () => { + // Arrange + routingOptions.defaultHash = true; + + // Act & Assert + expect(() => { + setRoutingOptions({ hashMode: 'multi' }); + }).toThrow("Using classic hash routing as default can only be done when 'hashMode' is set to 'single'."); + }); + + test.each([ + { hashMode: 'single' as const, defaultHash: false, scenario: 'single hash mode with defaultHash false' }, + { hashMode: 'single' as const, defaultHash: true, scenario: 'single hash mode with defaultHash true' }, + { hashMode: 'multi' as const, defaultHash: false, scenario: 'multi hash mode with defaultHash false' }, + { hashMode: 'multi' as const, defaultHash: 'namedHash', scenario: 'multi hash mode with named hash' } + ])("Should allow valid combination: $scenario .", ({ hashMode, defaultHash }) => { + // Arrange & Act & Assert + expect(() => { + setRoutingOptions({ hashMode, defaultHash }); + }).not.toThrow(); + + expect(routingOptions.hashMode).toBe(hashMode); + expect(routingOptions.defaultHash).toBe(defaultHash); + }); + }); + }); + describe('resetRoutingOptions', () => { test("Should reset all options to defaults when resetRoutingOptions is called.", () => { - // Arrange. - const original = structuredClone(routingOptions); + // Arrange - First reset to ensure we start from defaults, then capture the baseline + resetRoutingOptions(); + const expectedDefaults = structuredClone(routingOptions); + + // Modify all options to non-default values routingOptions.hashMode = 'multi'; routingOptions.defaultHash = true; routingOptions.disallowPathRouting = true; @@ -61,7 +183,7 @@ describe("options", () => { resetRoutingOptions(); // Assert. - expect(routingOptions).deep.equal(original); + expect(routingOptions).deep.equal(expectedDefaults); }); }); }); diff --git a/src/lib/kernel/options.ts b/src/lib/kernel/options.ts index 0ef4b3d..66d4421 100644 --- a/src/lib/kernel/options.ts +++ b/src/lib/kernel/options.ts @@ -31,6 +31,9 @@ export function setRoutingOptions(options?: Partial): vo if (routingOptions.hashMode === 'single' && typeof routingOptions.defaultHash === 'string') { throw new Error("Using a named hash path as the default path can only be done when 'hashMode' is set to 'multi'."); } + else if (routingOptions.hashMode === 'multi' && routingOptions.defaultHash === true) { + throw new Error("Using classic hash routing as default can only be done when 'hashMode' is set to 'single'."); + } } /**