Skip to content

Commit ec176a8

Browse files
authored
Constrain BaseController state to be valid JSON (#366)
The BaseController state is now constrained to valid JSON. Anything that can't be serialized (e.g. functions) or gets mutated during serialization (e.g. `undefined` gets converted to `null`) is disallowed. This should prevent an entire class of bugs resulting from unexpected changes when serializing state as JSON. For example, we serialize state when sending it over `postMessage`, and when persisting the state.
1 parent 0432bf8 commit ec176a8

File tree

1 file changed

+35
-4
lines changed

1 file changed

+35
-4
lines changed

src/BaseControllerV2.ts

+35-4
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,42 @@ enablePatches();
1111
*/
1212
export type Listener<T> = (state: T, patches: Patch[]) => void;
1313

14+
type primitive = null | boolean | number | string;
15+
16+
type DefinitelyNotJsonable = ((...args: any[]) => any) | undefined;
17+
18+
// Type copied from https://github.com/Microsoft/TypeScript/issues/1897#issuecomment-710744173
19+
export type IsJsonable<T> =
20+
// Check if there are any non-jsonable types represented in the union
21+
// Note: use of tuples in this first condition side-steps distributive conditional types
22+
// (see https://github.com/microsoft/TypeScript/issues/29368#issuecomment-453529532)
23+
[Extract<T, DefinitelyNotJsonable>] extends [never]
24+
? // Non-jsonable type union was found empty
25+
T extends primitive
26+
? // Primitive is acceptable
27+
T
28+
: // Otherwise check if array
29+
T extends (infer U)[]
30+
? // Arrays are special; just check array element type
31+
IsJsonable<U>[]
32+
: // Otherwise check if object
33+
// eslint-disable-next-line @typescript-eslint/ban-types
34+
T extends object
35+
? // It's an object
36+
{
37+
// Iterate over keys in object case
38+
[P in keyof T]: IsJsonable<T[P]>; // Recursive call for children
39+
}
40+
: // Otherwise any other non-object no bueno
41+
never
42+
: // Otherwise non-jsonable type union was found not empty
43+
never;
44+
1445
/**
1546
* Controller class that provides state management and subscriptions
1647
*/
1748
export class BaseController<S extends Record<string, unknown>> {
18-
private internalState: S;
49+
private internalState: IsJsonable<S>;
1950

2051
private internalListeners: Set<Listener<S>> = new Set();
2152

@@ -24,7 +55,7 @@ export class BaseController<S extends Record<string, unknown>> {
2455
*
2556
* @param state - Initial controller state
2657
*/
27-
constructor(state: S) {
58+
constructor(state: IsJsonable<S>) {
2859
this.internalState = state;
2960
}
3061

@@ -68,9 +99,9 @@ export class BaseController<S extends Record<string, unknown>> {
6899
* @param callback - Callback for updating state, passed a draft state
69100
* object. Return a new state object or mutate the draft to update state.
70101
*/
71-
protected update(callback: (state: Draft<S>) => void | S) {
102+
protected update(callback: (state: Draft<IsJsonable<S>>) => void | IsJsonable<S>) {
72103
const [nextState, patches] = produceWithPatches(this.internalState, callback);
73-
this.internalState = nextState as S;
104+
this.internalState = nextState as IsJsonable<S>;
74105
for (const listener of this.internalListeners) {
75106
listener(nextState as S, patches);
76107
}

0 commit comments

Comments
 (0)