Skip to content

Commit 13425bd

Browse files
authored
fix: Ensure the cleanup function from init() really rolls back everything (#60)
1 parent 06b791c commit 13425bd

File tree

8 files changed

+355
-15
lines changed

8 files changed

+355
-15
lines changed

src/lib/core/Logger.test.ts

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
import { describe, test, expect, beforeEach, afterEach, vi } from "vitest";
2-
import { logger, setLogger } from "./Logger.js";
2+
import { logger, resetLogger, setLogger } from "./Logger.js";
33
import type { ILogger } from "$lib/types.js";
44

55
describe("logger", () => {
6-
test("Should default to globalThis.console", () => {
7-
expect(logger).toBe(globalThis.console);
6+
test("Should default to offLogger (not console)", () => {
7+
expect(logger).not.toBe(globalThis.console);
8+
expect(logger.debug).toBeDefined();
9+
expect(logger.log).toBeDefined();
10+
expect(logger.warn).toBeDefined();
11+
expect(logger.error).toBeDefined();
812
});
913
});
1014

@@ -158,4 +162,45 @@ describe("setLogger", () => {
158162
expect(mockLogger.error).toHaveBeenCalledWith("error", { error: "object" });
159163
});
160164
});
165+
166+
describe("resetLogger", () => {
167+
test("Should reset logger to offLogger (default uninitialized state).", () => {
168+
// Arrange - Set logger to console
169+
setLogger(true);
170+
expect(logger).toBe(globalThis.console);
171+
172+
// Act
173+
resetLogger();
174+
175+
// Assert - Logger should be back to offLogger
176+
expect(logger).not.toBe(globalThis.console);
177+
expect(logger.debug).toBeDefined();
178+
expect(logger.log).toBeDefined();
179+
expect(logger.warn).toBeDefined();
180+
expect(logger.error).toBeDefined();
181+
});
182+
183+
test("Should reset logger from custom logger to offLogger.", () => {
184+
// Arrange - Set logger to custom logger
185+
const customLogger = {
186+
debug: vi.fn(),
187+
log: vi.fn(),
188+
warn: vi.fn(),
189+
error: vi.fn()
190+
};
191+
setLogger(customLogger);
192+
expect(logger).toBe(customLogger);
193+
194+
// Act
195+
resetLogger();
196+
197+
// Assert - Logger should be back to offLogger
198+
expect(logger).not.toBe(customLogger);
199+
expect(logger).not.toBe(globalThis.console);
200+
expect(logger.debug).toBeDefined();
201+
expect(logger.log).toBeDefined();
202+
expect(logger.warn).toBeDefined();
203+
expect(logger.error).toBeDefined();
204+
});
205+
});
161206
});

src/lib/core/Logger.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,15 @@ const offLogger: ILogger = {
1111
error: noop
1212
};
1313

14-
export let logger: ILogger = stockLogger;
14+
export let logger: ILogger = offLogger;
1515

1616
export function setLogger(newLogger: boolean | ILogger) {
1717
logger = newLogger === true ? stockLogger : (newLogger === false ? offLogger : newLogger);
18-
};
18+
}
19+
20+
/**
21+
* Resets the logger to the default uninitialized state (offLogger).
22+
*/
23+
export function resetLogger(): void {
24+
logger = offLogger;
25+
}

src/lib/core/options.test.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, test } from "vitest";
2-
import { routingOptions } from "./options.js";
2+
import { resetRoutingOptions, routingOptions } from "./options.js";
33

44
describe("options", () => {
55
test("The routing options' initial values are the expected ones.", () => {
@@ -94,4 +94,24 @@ describe("options", () => {
9494
// Restore original value
9595
routingOptions.implicitMode = originalValue;
9696
});
97+
98+
test("Should reset all options to defaults when resetRoutingOptions is called.", () => {
99+
// Arrange - Modify all options to non-default values
100+
routingOptions.full = true;
101+
routingOptions.hashMode = 'multi';
102+
routingOptions.implicitMode = 'hash';
103+
104+
// Verify they were changed
105+
expect(routingOptions.full).toBe(true);
106+
expect(routingOptions.hashMode).toBe('multi');
107+
expect(routingOptions.implicitMode).toBe('hash');
108+
109+
// Act
110+
resetRoutingOptions();
111+
112+
// Assert - Verify all options are back to defaults
113+
expect(routingOptions.full).toBe(false);
114+
expect(routingOptions.hashMode).toBe('single');
115+
expect(routingOptions.implicitMode).toBe('path');
116+
});
97117
});

src/lib/core/options.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,22 @@ export type RoutingOptions = {
4747
}
4848

4949
/**
50-
* Global routing options.
50+
* Default routing options used for rollback.
5151
*/
52-
export const routingOptions: Required<RoutingOptions> = {
52+
const defaultRoutingOptions: Required<RoutingOptions> = {
5353
full: false,
5454
hashMode: 'single',
5555
implicitMode: 'path'
5656
};
57+
58+
/**
59+
* Global routing options.
60+
*/
61+
export const routingOptions: Required<RoutingOptions> = structuredClone(defaultRoutingOptions);
62+
63+
/**
64+
* Resets routing options to their default values.
65+
*/
66+
export function resetRoutingOptions(): void {
67+
Object.assign(routingOptions, structuredClone(defaultRoutingOptions));
68+
}

src/lib/core/trace.svelte.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,24 @@ export function getAllChildRouters(parent: RouterEngine) {
7474
}
7575

7676
/**
77-
* Tracing options that can be set during library initialization.
77+
* Default tracing options used for rollback.
7878
*/
79-
export const traceOptions: TraceOptions = {
79+
const defaultTraceOptions: TraceOptions = {
8080
routerHierarchy: false,
8181
};
8282

83+
/**
84+
* Tracing options that can be set during library initialization.
85+
*/
86+
export const traceOptions: TraceOptions = structuredClone(defaultTraceOptions);
87+
8388
export function setTraceOptions(options?: typeof traceOptions) {
8489
Object.assign(traceOptions, options);
8590
}
91+
92+
/**
93+
* Resets tracing options to their default values.
94+
*/
95+
export function resetTraceOptions(): void {
96+
Object.assign(traceOptions, structuredClone(defaultTraceOptions));
97+
}

src/lib/core/trace.test.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterAll, beforeAll, beforeEach, describe, expect, test, vi } from "vitest";
2-
import { getAllChildRouters, setTraceOptions, traceOptions } from "./trace.svelte.js";
2+
import { getAllChildRouters, resetTraceOptions, setTraceOptions, traceOptions } from "./trace.svelte.js";
33
import { RouterEngine } from "./RouterEngine.svelte.js";
44
import { init } from "$lib/index.js";
55

@@ -29,6 +29,32 @@ describe('setTraceOptions', () => {
2929
});
3030
});
3131

32+
describe('resetTraceOptions', () => {
33+
test("Should reset trace options to defaults.", () => {
34+
// Arrange - Set to non-default value
35+
setTraceOptions({ routerHierarchy: true });
36+
expect(traceOptions.routerHierarchy).toBe(true);
37+
38+
// Act
39+
resetTraceOptions();
40+
41+
// Assert - Should be back to default
42+
expect(traceOptions.routerHierarchy).toBe(false);
43+
});
44+
45+
test("Should reset trace options when already at defaults.", () => {
46+
// Arrange - Ensure it's already at default
47+
resetTraceOptions();
48+
expect(traceOptions.routerHierarchy).toBe(false);
49+
50+
// Act
51+
resetTraceOptions();
52+
53+
// Assert - Should still be at default
54+
expect(traceOptions.routerHierarchy).toBe(false);
55+
});
56+
});
57+
3258
describe('registerRouter', () => {
3359
let cleanup: () => void;
3460
beforeAll(() => {

0 commit comments

Comments
 (0)