Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add controller messaging system #377

Merged
merged 6 commits into from
Mar 11, 2021
Merged

Add controller messaging system #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4699711
Add controller messaging system
Gudahtt Mar 3, 2021
85fc3ef
Remove `subscribe` and `unsubscribe` BaseController methods
Gudahtt Mar 8, 2021
ed6488f
Rename `state-change` event to `stateChange`
Gudahtt Mar 8, 2021
cc8f81d
Add JSDoc comments, and rename type parameters
Gudahtt Mar 8, 2021
3ed24a6
Add JSDOc comment for ControllerMessenger class
Gudahtt Mar 9, 2021
1671284
Remove unused type exports
Gudahtt Mar 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 106 additions & 32 deletions src/BaseControllerV2.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,27 @@
import type { Draft } from 'immer';
import type { Draft, Patch } from 'immer';
import * as sinon from 'sinon';

import { BaseController, getAnonymizedState, getPersistentState } from './BaseControllerV2';
import { ControllerMessenger } from './ControllerMessenger';

type MockControllerState = {
type CountControllerState = {
count: number;
};

const mockControllerStateMetadata = {
type CountControllerEvent = {
type: `CountController:stateChange`;
payload: [CountControllerState, Patch[]];
};

const CountControllerStateMetadata = {
count: {
persist: true,
anonymous: true,
},
};

class MockController extends BaseController<MockControllerState> {
update(callback: (state: Draft<MockControllerState>) => void | MockControllerState) {
class MockController extends BaseController<'CountController', CountControllerState> {
update(callback: (state: Draft<CountControllerState>) => void | CountControllerState) {
super.update(callback);
}

Expand All @@ -26,27 +32,51 @@ class MockController extends BaseController<MockControllerState> {

describe('BaseController', () => {
it('should set initial state', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);

expect(controller.state).toEqual({ count: 0 });
});

it('should set initial schema', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);

expect(controller.metadata).toEqual(mockControllerStateMetadata);
expect(controller.metadata).toEqual(CountControllerStateMetadata);
});

it('should not allow mutating state directly', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);

expect(() => {
controller.state = { count: 1 };
}).toThrow();
});

it('should allow updating state by modifying draft', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);

controller.update((draft) => {
draft.count += 1;
Expand All @@ -56,7 +86,13 @@ describe('BaseController', () => {
});

it('should allow updating state by return a value', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);

controller.update(() => {
return { count: 1 };
Expand All @@ -66,7 +102,13 @@ describe('BaseController', () => {
});

it('should throw an error if update callback modifies draft and returns value', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);

expect(() => {
controller.update((draft) => {
Expand All @@ -77,12 +119,18 @@ describe('BaseController', () => {
});

it('should inform subscribers of state changes', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);
const listener1 = sinon.stub();
const listener2 = sinon.stub();

controller.subscribe(listener1);
controller.subscribe(listener2);
controllerMessenger.subscribe('CountController:stateChange', listener1);
controllerMessenger.subscribe('CountController:stateChange', listener2);
controller.update(() => {
return { count: 1 };
});
Expand All @@ -94,11 +142,18 @@ describe('BaseController', () => {
});

it('should inform a subscriber of each state change once even after multiple subscriptions', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);
const listener1 = sinon.stub();

controller.subscribe(listener1);
controller.subscribe(listener1);
controllerMessenger.subscribe('CountController:stateChange', listener1);
controllerMessenger.subscribe('CountController:stateChange', listener1);

controller.update(() => {
return { count: 1 };
});
Expand All @@ -108,11 +163,17 @@ describe('BaseController', () => {
});

it('should no longer inform a subscriber about state changes after unsubscribing', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);
const listener1 = sinon.stub();

controller.subscribe(listener1);
controller.unsubscribe(listener1);
controllerMessenger.subscribe('CountController:stateChange', listener1);
controllerMessenger.unsubscribe('CountController:stateChange', listener1);
controller.update(() => {
return { count: 1 };
});
Expand All @@ -121,35 +182,48 @@ describe('BaseController', () => {
});

it('should no longer inform a subscriber about state changes after unsubscribing once, even if they subscribed many times', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);
const listener1 = sinon.stub();

controller.subscribe(listener1);
controller.subscribe(listener1);
controller.unsubscribe(listener1);
controllerMessenger.subscribe('CountController:stateChange', listener1);
controllerMessenger.subscribe('CountController:stateChange', listener1);
controllerMessenger.unsubscribe('CountController:stateChange', listener1);
controller.update(() => {
return { count: 1 };
});

expect(listener1.callCount).toEqual(0);
});

it('should allow unsubscribing listeners who were never subscribed', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
it('should throw when unsubscribing listener who was never subscribed', () => {
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
new MockController(controllerMessenger, 'CountController', { count: 0 }, CountControllerStateMetadata);
const listener1 = sinon.stub();

expect(() => {
controller.unsubscribe(listener1);
}).not.toThrow();
controllerMessenger.unsubscribe('CountController:stateChange', listener1);
}).toThrow();
});

it('should no longer update subscribers after being destroyed', () => {
const controller = new MockController({ count: 0 }, mockControllerStateMetadata);
const controllerMessenger = new ControllerMessenger<never, CountControllerEvent>();
const controller = new MockController(
controllerMessenger,
'CountController',
{ count: 0 },
CountControllerStateMetadata,
);
const listener1 = sinon.stub();
const listener2 = sinon.stub();

controller.subscribe(listener1);
controller.subscribe(listener2);
controllerMessenger.subscribe('CountController:stateChange', listener1);
controllerMessenger.subscribe('CountController:stateChange', listener2);
controller.destroy();
controller.update(() => {
return { count: 1 };
Expand Down
42 changes: 17 additions & 25 deletions src/BaseControllerV2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { enablePatches, produceWithPatches } from 'immer';
// eslint-disable-next-line no-duplicate-imports
import type { Draft, Patch } from 'immer';

import type { ControllerMessenger } from './ControllerMessenger';

enablePatches();

/**
Expand Down Expand Up @@ -96,21 +98,31 @@ type Json = null | boolean | number | string | Json[] | { [prop: string]: Json }
/**
* Controller class that provides state management, subscriptions, and state metadata
*/
export class BaseController<S extends Record<string, unknown>> {
export class BaseController<N extends string, S extends Record<string, unknown>> {
private internalState: IsJsonable<S>;

private internalListeners: Set<Listener<S>> = new Set();
private messagingSystem: ControllerMessenger<never, { type: `${N}:stateChange`; payload: [S, Patch[]] }>;

private name: N;

public readonly metadata: StateMetadata<S>;

/**
* Creates a BaseController instance.
*
* @param messagingSystem - Controller messaging system
* @param state - Initial controller state
* @param metadata - State metadata, describing how to "anonymize" the state,
* and which parts should be persisted.
*/
constructor(state: IsJsonable<S>, metadata: StateMetadata<S>) {
constructor(
messagingSystem: ControllerMessenger<never, { type: `${N}:stateChange`; payload: [S, Patch[]] }>,
name: N,
state: IsJsonable<S>,
metadata: StateMetadata<S>,
) {
this.messagingSystem = messagingSystem;
this.name = name;
this.internalState = state;
this.metadata = metadata;
}
Expand All @@ -128,24 +140,6 @@ export class BaseController<S extends Record<string, unknown>> {
throw new Error(`Controller state cannot be directly mutated; use 'update' method instead.`);
}

/**
* Adds new listener to be notified of state changes
*
* @param listener - Callback triggered when state changes
*/
subscribe(listener: Listener<S>) {
this.internalListeners.add(listener);
}

/**
* Removes existing listener from receiving state changes
*
* @param listener - Callback to remove
*/
unsubscribe(listener: Listener<S>) {
this.internalListeners.delete(listener);
}

/**
* Updates controller state. Accepts a callback that is passed a draft copy
* of the controller state. If a value is returned, it is set as the new
Expand All @@ -158,9 +152,7 @@ export class BaseController<S extends Record<string, unknown>> {
protected update(callback: (state: Draft<IsJsonable<S>>) => void | IsJsonable<S>) {
const [nextState, patches] = produceWithPatches(this.internalState, callback);
this.internalState = nextState as IsJsonable<S>;
for (const listener of this.internalListeners) {
listener(nextState as S, patches);
}
this.messagingSystem.publish(`${this.name}:stateChange` as `${N}:stateChange`, nextState as S, patches);
}

/**
Expand All @@ -173,7 +165,7 @@ export class BaseController<S extends Record<string, unknown>> {
* listeners from being garbage collected.
*/
protected destroy() {
this.internalListeners.clear();
this.messagingSystem.clearEventSubscriptions(`${this.name}:stateChange` as `${N}:stateChange`);
}
}

Expand Down
Loading