Skip to content

Commit 7862dc2

Browse files
committed
Constrain BaseController state to be valid JSON
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 05af913 commit 7862dc2

File tree

1 file changed

+40
-4
lines changed

1 file changed

+40
-4
lines changed

src/BaseControllerV2.ts

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,47 @@ enablePatches();
1111
*/
1212
export type Listener<T> = (state: T, patches: Patch[]) => void;
1313

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

2056
private internalListeners: Set<Listener<S>> = new Set();
2157

@@ -24,7 +60,7 @@ export class BaseController<S extends Record<string, unknown>> {
2460
*
2561
* @param state - Initial controller state
2662
*/
27-
constructor(state: S) {
63+
constructor(state: IsJsonable<S>) {
2864
this.internalState = state;
2965
}
3066

@@ -68,9 +104,9 @@ export class BaseController<S extends Record<string, unknown>> {
68104
* @param callback - Callback for updating state, passed a draft state
69105
* object. Return a new state object or mutate the draft to update state.
70106
*/
71-
protected update(callback: (state: Draft<S>) => void | S) {
107+
protected update(callback: (state: Draft<IsJsonable<S>>) => void | IsJsonable<S>) {
72108
const [nextState, patches] = produceWithPatches(this.internalState, callback);
73-
this.internalState = nextState as S;
109+
this.internalState = nextState as IsJsonable<S>;
74110
for (const listener of this.internalListeners) {
75111
listener(nextState as S, patches);
76112
}

0 commit comments

Comments
 (0)