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.

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

Factor out a MutableState interface from the store and provide an ImmutableJS implementation #242

Merged
merged 13 commits into from
Feb 26, 2019
7 changes: 6 additions & 1 deletion intern.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@
"shimPath": "./dist/dev/src/shim/util/amd.js",
"packages": [
{ "name": "src", "location": "dist/dev/src" },
{ "name": "tests", "location": "dist/dev/tests" }
{ "name": "tests", "location": "dist/dev/tests" },
{
"name": "immutable",
"location": "./node_modules/immutable/dist",
"main": "immutable.js"
}
],
"map": {
"*": {
Expand Down
5 changes: 5 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
"css-select-umd": "1.3.0-rc0",
"diff": "3.5.0",
"globalize": "1.4.0",
"immutable": "3.8.2",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include immutable as a dependency, or make it a dev dependency and also an optional dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it an optional and dev dependency. In the code I just wrapped the require in a try. I'm assuming that we expect that module will fail if Immutable isn't installed, but I'm not sure how we want to handle that. I didn't see anywhere else we're using optional dependencies so let me know if that's the wrong approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should put something in the README to say that they would need to install immutable.... Or maybe having it as a dev isn't the end of the world... it's not going to affect anything unless they use it anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your call. Since it should be left out if they're not using it I'd be fine with it being a dependency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah let's put it in the deps again. sorry!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. Reverted

"intersection-observer": "0.4.2",
"pepjs": "0.4.2",
"resize-observer-polyfill": "1.5.0",
Expand Down
25 changes: 24 additions & 1 deletion src/stores/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ An application store for dojo.
- [Transforming Executor Arguments](#transforming-executor-arguments)
- [Optimistic Update Pattern](#optimistic-update-pattern)
- [Executing Concurrent Commands](#executing-concurrent-commands)
- [Providing An Alternative State Implementation](#providing-an-alternative-state-implementation)
- [Middleware](#middleware)
- [After Middleware](#after-middleware)
- [Before Middleware](#before-middleware)
Expand Down Expand Up @@ -220,7 +221,7 @@ and will immediately throw an error.
function calculateCountsCommand = createCommand(({ state }) => {
const todos = state.todos;
const completedTodos = todos.filter((todo: any) => todo.completed);

state.activeCount = todos.length - completedTodos.length;
state.completedCount = completedTodos.length;
});
Expand Down Expand Up @@ -567,6 +568,28 @@ In this example, `commandOne` is executed, then both `concurrentCommandOne` and

**Note:** Concurrent commands are always assumed to be asynchronous and resolved using `Promise.all`.

### Providing an alternative State implementation

Processing operations and updating the store state is handled by an implementation of the `MutableState` interface
defined in `Store.ts`. This interface defines four methods necessary to properly apply operations to the state.

- `get<S>(path: Path<M, S>): S` Takes a `Path` object and returns the value in the current state that that path points to
- `at<S extends Path<M, Array<any>>>(path: S, index: number): Path<M, S['value'][0]>` Returns a `Path` object that
points to the provided `index` in the array at the provided `path`
- `path: StatePaths<M>` A typesafe way to generate a `Path` object for a given path in the state
- `apply(operations: PatchOperation<T>[]): PatchOperation<T>[]` Apply the provided operations to the current state

The default state implementation is reasonably optimized and in most circumstances will be sufficient.
If a particular use case merits an alternative implementation it can be provided to the store constructor

```ts
const store = new Store({ state: myStateImpl });
```

#### ImmutableState

An implementation of the `MutableState` interface that leverages [Immutable](https://github.com/immutable-js/immutable-js) under the hood is provided as an example. This implementation may provide better performance if there are frequent, deep updates to the store's state, but performance should be tested and verified for your app before switching to this implementation.

## Middleware

Middleware provides a hook to apply generic/global functionality across multiple or all processes used within an application. Middleware is a function that returns an object with optional `before` and `after` callback functions.
Expand Down
105 changes: 75 additions & 30 deletions src/stores/Store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,19 @@ function isString(segment?: string): segment is string {
return typeof segment === 'string';
}

/**
* Application state store
*/
export class Store<T = any> extends Evented implements State<T> {
export interface MutableState<T = any> extends State<T> {
/**
* Applies store operations to state and returns the undo operations
*/
apply(operations: PatchOperation<T>[]): PatchOperation<T>[];
}

export class DefaultState<T = any> implements MutableState<T> {
/**
* The private state object
*/
private _state = {} as T;

private _changePaths = new Map<string, OnChangeValue>();

private _callbackId = 0;

/**
* Returns the state at a specific pointer path location.
*/
Expand All @@ -151,13 +151,10 @@ export class Store<T = any> extends Evented implements State<T> {
/**
* Applies store operations to state and returns the undo operations
*/
public apply = (operations: PatchOperation<T>[], invalidate: boolean = false): PatchOperation<T>[] => {
public apply = (operations: PatchOperation<T>[]): PatchOperation<T>[] => {
const patch = new Patch(operations);
const patchResult = patch.apply(this._state);
this._state = patchResult.object;
if (invalidate) {
this.invalidate();
}
return patchResult.undoOperations;
};

Expand All @@ -172,6 +169,67 @@ export class Store<T = any> extends Evented implements State<T> {
};
};

public path: State<T>['path'] = (path: string | Path<T, any>, ...segments: (string | undefined)[]) => {
if (typeof path === 'string') {
segments = [path, ...segments];
} else {
segments = [...new Pointer(path.path).segments, ...segments];
}

const stringSegments = segments.filter<string>(isString);
const hasMultipleSegments = stringSegments.length > 1;
const pointer = new Pointer(hasMultipleSegments ? stringSegments : stringSegments[0] || '');

return {
path: pointer.path,
state: this._state,
value: pointer.get(this._state)
};
};
}

/**
* Application state store
*/
export class Store<T = any> extends Evented implements MutableState<T> {
private _adapter: MutableState<T> = new DefaultState<T>();

private _changePaths = new Map<string, OnChangeValue>();

private _callbackId = 0;

/**
* Returns the state at a specific pointer path location.
*/
public get = <U = any>(path: Path<T, U>): U => {
return this._adapter.get(path);
};

constructor(options?: { state?: MutableState<T> }) {
super();
if (options && options.state) {
this._adapter = options.state;
this.path = this._adapter.path.bind(this._adapter);
}
}

/**
* Applies store operations to state and returns the undo operations
*/
public apply = (operations: PatchOperation<T>[], invalidate: boolean = false): PatchOperation<T>[] => {
const result = this._adapter.apply(operations);

if (invalidate) {
this.invalidate();
}

return result;
};

public at = <U = any>(path: Path<T, Array<U>>, index: number): Path<T, U> => {
return this._adapter.at(path, index);
};

public onChange = <U = any>(paths: Path<T, U> | Path<T, U>[], callback: () => void) => {
const callbackId = this._callbackId;
if (!Array.isArray(paths)) {
Expand Down Expand Up @@ -206,7 +264,10 @@ export class Store<T = any> extends Evented implements State<T> {
const callbackIdsCalled: number[] = [];
this._changePaths.forEach((value: OnChangeValue, path: string) => {
const { previousValue, callbacks } = value;
const newValue = new Pointer(path).get(this._state);
const pointer = new Pointer(path);
const newValue = pointer.segments.length
? this._adapter.path(pointer.segments[0] as keyof T, ...pointer.segments.slice(1)).value
: undefined;
if (previousValue !== newValue) {
this._changePaths.set(path, { callbacks, previousValue: newValue });
callbacks.forEach((callbackItem) => {
Expand All @@ -228,23 +289,7 @@ export class Store<T = any> extends Evented implements State<T> {
this.emit({ type: 'invalidate' });
}

public path: State<T>['path'] = (path: string | Path<T, any>, ...segments: (string | undefined)[]) => {
if (typeof path === 'string') {
segments = [path, ...segments];
} else {
segments = [...new Pointer(path.path).segments, ...segments];
}

const stringSegments = segments.filter<string>(isString);
const hasMultipleSegments = stringSegments.length > 1;
const pointer = new Pointer(hasMultipleSegments ? stringSegments : stringSegments[0] || '');

return {
path: pointer.path,
state: this._state,
value: pointer.get(this._state)
};
};
public path: State<T>['path'] = this._adapter.path.bind(this._adapter);
}

export default Store;
Loading