Skip to content

Commit

Permalink
Constrain BaseController state to be valid JSON
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gudahtt committed Feb 25, 2021
1 parent 0d03fa4 commit 91e9715
Showing 1 changed file with 35 additions and 4 deletions.
39 changes: 35 additions & 4 deletions src/BaseControllerV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,42 @@ enablePatches();
*/
export type Listener<T> = (state: T, patches: Patch[]) => void;

type primitive = null | boolean | number | string;

type DefinitelyNotJsonable = ((...args: any[]) => any) | undefined;

// Type copied from https://github.com/Microsoft/TypeScript/issues/1897#issuecomment-710744173
export type IsJsonable<T> =
// Check if there are any non-jsonable types represented in the union
// Note: use of tuples in this first condition side-steps distributive conditional types
// (see https://github.com/microsoft/TypeScript/issues/29368#issuecomment-453529532)
[Extract<T, DefinitelyNotJsonable>] extends [never]
? // Non-jsonable type union was found empty
T extends primitive
? // Primitive is acceptable
T
: // Otherwise check if array
T extends (infer U)[]
? // Arrays are special; just check array element type
IsJsonable<U>[]
: // Otherwise check if object
// eslint-disable-next-line @typescript-eslint/ban-types
T extends object
? // It's an object
{
// Iterate over keys in object case
[P in keyof T]: IsJsonable<T[P]>; // Recursive call for children
}
: // Otherwise any other non-object no bueno
never
: // Otherwise non-jsonable type union was found not empty
never;

/**
* Controller class that provides state management and subscriptions
*/
export class BaseController<S extends Record<string, unknown>> {
private internalState: S;
private internalState: IsJsonable<S>;

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

Expand All @@ -24,7 +55,7 @@ export class BaseController<S extends Record<string, unknown>> {
*
* @param state - Initial controller state
*/
constructor(state: S) {
constructor(state: IsJsonable<S>) {
this.internalState = state;
}

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

0 comments on commit 91e9715

Please sign in to comment.