From 9810ba5a2ee8558f4693834ae8172803710f6a4b Mon Sep 17 00:00:00 2001 From: Bradley Maier Date: Mon, 3 Feb 2020 15:18:04 -0800 Subject: [PATCH 1/2] Make indices numbers again --- src/core/middleware/store.ts | 5 ++-- src/stores/Store.ts | 26 +++++++++++-------- src/stores/process.ts | 2 +- src/stores/state/ImmutableState.ts | 20 +++++++++------ src/stores/state/Patch.ts | 14 +++++----- src/stores/state/Pointer.ts | 41 +++++++++++++++++++----------- 6 files changed, 64 insertions(+), 44 deletions(-) diff --git a/src/core/middleware/store.ts b/src/core/middleware/store.ts index 7c1799e39..cf4ca232f 100644 --- a/src/core/middleware/store.ts +++ b/src/core/middleware/store.ts @@ -30,12 +30,13 @@ export const createStoreMiddleware = (initial?: (store: Store) => vo }; return { get(path: Path): U { - if (registeredPaths.indexOf(path.path) === -1) { + const stringPath = Array.isArray(path.path) ? path.path.join('/') : String(path.path); + if (registeredPaths.indexOf(stringPath) === -1) { const handle = store.onChange(path, () => { invalidator(); }); handles.push(() => handle.remove()); - registeredPaths.push(path.path); + registeredPaths.push(stringPath); } return store.get(path); }, diff --git a/src/stores/Store.ts b/src/stores/Store.ts index 4e26b0e16..de0143ea4 100644 --- a/src/stores/Store.ts +++ b/src/stores/Store.ts @@ -9,7 +9,7 @@ import Map from '../shim/Map'; * */ export interface Path { - path: string; + path: string | number | (string | number)[]; state: M; value: T; } @@ -24,7 +24,6 @@ export interface State { at>>(path: S, index: number): Path; path: StatePaths; } - export interface StatePaths { >(path: Path, a: P0): Path[P0]>; [P0]>(path: Path, a: P0, b: P1): Path< @@ -108,8 +107,9 @@ interface OnChangeValue { previousValue: any; } -function isString(segment?: string): segment is string { - return typeof segment === 'string'; +function isStringOrNumber(segment?: string | number): segment is string | number { + const type = typeof segment; + return type === 'string' || type === 'number'; } export interface MutableState extends State { @@ -153,14 +153,17 @@ export class DefaultState implements MutableState { }; }; - public path: State['path'] = (path: string | Path, ...segments: (string | undefined)[]) => { - if (typeof path === 'string') { + public path: State['path'] = ( + path: string | number | Path, + ...segments: (string | number | undefined)[] + ) => { + if (typeof path === 'string' || typeof path === 'number') { segments = [path, ...segments]; } else { segments = [...new Pointer(path.path).segments, ...segments]; } - const stringSegments = segments.filter(isString); + const stringSegments = segments.filter(isStringOrNumber); const hasMultipleSegments = stringSegments.length > 1; const pointer = new Pointer(hasMultipleSegments ? stringSegments : stringSegments[0] || ''); @@ -224,7 +227,9 @@ export class Store extends Evented implements MutableState { return { remove: () => { (paths as Path[]).forEach((path) => { - const onChange = this._changePaths.get(path.path); + const onChange = this._changePaths.get( + Array.isArray(path.path) ? path.path.join('/') : String(path.path) + ); if (onChange) { onChange.callbacks = onChange.callbacks.filter((callback) => { return callback.callbackId !== callbackId; @@ -236,12 +241,13 @@ export class Store extends Evented implements MutableState { }; private _addOnChange = (path: Path, callback: () => void, callbackId: number): void => { - let changePaths = this._changePaths.get(path.path); + const stringPath = Array.isArray(path.path) ? path.path.join('/') : String(path.path); + let changePaths = this._changePaths.get(stringPath); if (!changePaths) { changePaths = { callbacks: [], previousValue: this.get(path) }; } changePaths.callbacks.push({ callbackId, callback }); - this._changePaths.set(path.path, changePaths); + this._changePaths.set(stringPath, changePaths); }; private _runOnChanges() { diff --git a/src/stores/process.ts b/src/stores/process.ts index 2e4d60a41..a7289fe40 100644 --- a/src/stores/process.ts +++ b/src/stores/process.ts @@ -242,7 +242,7 @@ export function processExecutor( const createHandler = (partialPath?: Path) => ({ get(obj: any, prop: string): any { const fullPath = partialPath ? path(partialPath, prop) : path(prop as keyof T); - const stringPath = fullPath.path; + const stringPath = Array.isArray(fullPath.path) ? fullPath.path.join('/') : String(fullPath.path); if (typeof prop === 'symbol' && prop === valueSymbol) { return proxied.get(stringPath); diff --git a/src/stores/state/ImmutableState.ts b/src/stores/state/ImmutableState.ts index f13e9447b..78559f6ad 100644 --- a/src/stores/state/ImmutableState.ts +++ b/src/stores/state/ImmutableState.ts @@ -11,8 +11,9 @@ import { Map, List } from 'immutable'; import { getFriendlyDifferenceMessage, isEqual } from './compare'; -function isString(segment?: string): segment is string { - return typeof segment === 'string'; +function isStringOrNumber(segment?: string | number): segment is string | number { + const type = typeof segment; + return type === 'string' || type === 'number'; } function isList(value?: any): value is List { @@ -88,14 +89,17 @@ export class ImmutableState implements MutableState { }; }; - public path: State['path'] = (path: string | Path, ...segments: (string | undefined)[]) => { - if (typeof path === 'string') { + public path: State['path'] = ( + path: string | number | Path, + ...segments: (string | number | undefined)[] + ) => { + if (typeof path === 'string' || typeof path === 'number') { segments = [path, ...segments]; } else { segments = [...new Pointer(path.path).segments, ...segments]; } - const stringSegments = segments.filter(isString); + const stringSegments = segments.filter(isStringOrNumber); const hasMultipleSegments = stringSegments.length > 1; const pointer = new Pointer(hasMultipleSegments ? stringSegments : stringSegments[0] || ''); let value = this._state.getIn(pointer.segments); @@ -157,7 +161,7 @@ export class ImmutableState implements MutableState { return undoOperations; } - private setIn(segments: string[], value: any, state: Map, add = false) { + private setIn(segments: (string | number)[], value: any, state: Map, add = false) { const updated = this.set(segments, value, state, add); if (updated) { return updated; @@ -171,7 +175,7 @@ export class ImmutableState implements MutableState { } const value = state.getIn([...segments.slice(0, index), segment]); if (!value || !(value instanceof List || value instanceof Map)) { - if (!isNaN(nextSegment) && !isNaN(parseInt(nextSegment, 0))) { + if (typeof nextSegment === 'number') { map = map.setIn([...segments.slice(0, index), segment], List()); } else { map = map.setIn([...segments.slice(0, index), segment], Map()); @@ -183,7 +187,7 @@ export class ImmutableState implements MutableState { return this.set(segments, value, state, add) || state; } - private set(segments: string[], value: any, state: Map, add = false) { + private set(segments: (string | number)[], value: any, state: Map, add = false) { if (typeof value === 'object' && value != null) { if (Array.isArray(value)) { value = List(value); diff --git a/src/stores/state/Patch.ts b/src/stores/state/Patch.ts index d52c153c9..f205a2f5d 100644 --- a/src/stores/state/Patch.ts +++ b/src/stores/state/Patch.ts @@ -44,9 +44,8 @@ export interface PatchResult { } function add(pointerTarget: PointerTarget, value: any): any { - let index = parseInt(pointerTarget.segment, 10); - if (Array.isArray(pointerTarget.target) && !isNaN(index)) { - pointerTarget.target.splice(index, 0, value); + if (Array.isArray(pointerTarget.target) && typeof pointerTarget.segment === 'number') { + pointerTarget.target.splice(pointerTarget.segment, 0, value); } else { pointerTarget.target[pointerTarget.segment] = value; } @@ -54,9 +53,8 @@ function add(pointerTarget: PointerTarget, value: any): any { } function replace(pointerTarget: PointerTarget, value: any): any { - let index = parseInt(pointerTarget.segment, 10); - if (Array.isArray(pointerTarget.target) && !isNaN(index)) { - pointerTarget.target.splice(index, 1, value); + if (Array.isArray(pointerTarget.target) && typeof pointerTarget.segment === 'number') { + pointerTarget.target.splice(pointerTarget.segment, 1, value); } else { pointerTarget.target[pointerTarget.segment] = value; } @@ -64,8 +62,8 @@ function replace(pointerTarget: PointerTarget, value: any): any { } function remove(pointerTarget: PointerTarget): any { - if (Array.isArray(pointerTarget.target)) { - pointerTarget.target.splice(parseInt(pointerTarget.segment, 10), 1); + if (Array.isArray(pointerTarget.target) && typeof pointerTarget.segment === 'number') { + pointerTarget.target.splice(pointerTarget.segment, 1); } else { delete pointerTarget.target[pointerTarget.segment]; } diff --git a/src/stores/state/Pointer.ts b/src/stores/state/Pointer.ts index 8823b8f08..c79a127bd 100644 --- a/src/stores/state/Pointer.ts +++ b/src/stores/state/Pointer.ts @@ -1,18 +1,23 @@ -export function decode(segment: string) { - return segment.replace(/~1/g, '/').replace(/~0/g, '~'); +export function decode(segment: string | number) { + return typeof segment === 'number' ? segment : segment.replace(/~1/g, '/').replace(/~0/g, '~'); } -function encode(segment: string) { - return segment.replace(/~/g, '~0').replace(/\//g, '~1'); +function encode(segment: string | number) { + return typeof segment === 'number' ? segment : segment.replace(/~/g, '~0').replace(/\//g, '~1'); } export interface PointerTarget { object: any; target: any; - segment: string; + segment: string | number; } -export function walk(segments: string[], object: any, clone = true, continueOnUndefined = true): PointerTarget { +export function walk( + segments: (string | number)[], + object: any, + clone = true, + continueOnUndefined = true +): PointerTarget { if (clone) { object = { ...object }; } @@ -27,7 +32,7 @@ export function walk(segments: string[], object: any, clone = true, continueOnUn return pointerTarget; } if (Array.isArray(pointerTarget.target) && segment === '-') { - segment = String(pointerTarget.target.length - 1); + segment = pointerTarget.target.length - 1; } if (index + 1 < segments.length) { const nextSegment: any = segments[index + 1]; @@ -43,10 +48,10 @@ export function walk(segments: string[], object: any, clone = true, continueOnUn target = [...target]; } else if (typeof target === 'object') { target = { ...target }; - } else if (isNaN(nextSegment) || isNaN(parseInt(nextSegment, 0))) { - target = {}; - } else { + } else if (typeof nextSegment === 'number') { target = []; + } else { + target = {}; } pointerTarget.target[segment] = target; pointerTarget.target = target; @@ -61,22 +66,28 @@ export function walk(segments: string[], object: any, clone = true, continueOnUn } export class Pointer { - private readonly _segments: string[]; + private readonly _segments: (string | number)[]; - constructor(segments: string | string[]) { + constructor(segments: number | string | (string | number)[]) { if (Array.isArray(segments)) { this._segments = segments; } else { - this._segments = (segments[0] === '/' ? segments : `/${segments}`).split('/'); + this._segments = + typeof segments === 'string' + ? (segments[0] === '/' ? segments : `/${segments}`).split('/') + : [segments]; this._segments.shift(); } - if (segments.length === 0 || ((segments.length === 1 && segments[0] === '/') || segments[0] === '')) { + if ( + typeof segments !== 'number' && + (segments.length === 0 || ((segments.length === 1 && segments[0] === '/') || segments[0] === '')) + ) { throw new Error('Access to the root is not supported.'); } this._segments = this._segments.map(decode); } - public get segments(): string[] { + public get segments(): (string | number)[] { return this._segments; } From d17f38e87382e6c0fe625170e219fec66437f630 Mon Sep 17 00:00:00 2001 From: Bradley Maier Date: Mon, 3 Feb 2020 15:18:36 -0800 Subject: [PATCH 2/2] Make tests pass again --- tests/stores/unit/process.ts | 17 +++++++++++++---- tests/stores/unit/state/ImmutableState.ts | 10 +++++----- tests/stores/unit/state/Patch.ts | 4 ++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/tests/stores/unit/process.ts b/tests/stores/unit/process.ts index ca35bd78d..f92b1f4db 100644 --- a/tests/stores/unit/process.ts +++ b/tests/stores/unit/process.ts @@ -175,6 +175,15 @@ const tests = (stateType: string, state?: () => MutableState) => { assert.strictEqual(foobar, 'foo/bar'); }); + it('handles array indices in `path`', () => { + const store = new Store<{ foo: { bar: string }[] }>(); + store.apply([add(store.path('foo'), [{ bar: 'baz' }])]); + const foo = store.get(store.path('foo')); + const foobar = store.get(store.path('foo', 0, 'bar')); + assert.deepEqual(foo, [{ bar: 'baz' }]); + assert.strictEqual(foobar, 'baz'); + }); + it('should handle optional properties for updates', () => { type StateType = { a?: { b?: string }; foo?: number; bar: string }; const createCommand = createCommandFactory(); @@ -649,7 +658,7 @@ const tests = (stateType: string, state?: () => MutableState) => { const paths = result.operations.map((operation) => operation.path.path); const logs = result.get(store.path('logs')) || []; - result.apply([{ op: OperationType.ADD, path: new Pointer(`/logs/${logs.length}`), value: paths }]); + result.apply([{ op: OperationType.ADD, path: new Pointer(['logs', logs.length]), value: paths }]); } }); @@ -682,7 +691,7 @@ const tests = (stateType: string, state?: () => MutableState) => { store.apply([ { op: OperationType.ADD, - path: new Pointer(`/initLogs/${initLog.length}`), + path: new Pointer(['initLogs', initLog.length]), value: 'initial value' } ]); @@ -712,7 +721,7 @@ const tests = (stateType: string, state?: () => MutableState) => { const paths = result.operations.map((operation) => operation.path.path); const logs = result.get(store.path('logs')) || []; - result.apply([{ op: OperationType.ADD, path: new Pointer(`/logs/${logs.length}`), value: paths }]); + result.apply([{ op: OperationType.ADD, path: new Pointer(['logs', logs.length]), value: paths }]); } }); @@ -764,7 +773,7 @@ const tests = (stateType: string, state?: () => MutableState) => { const paths = result.operations.map((operation) => operation.path.path); const logs = result.get(store.path('logs')) || []; - result.apply([{ op: OperationType.ADD, path: new Pointer(`/logs/${logs.length}`), value: paths }]); + result.apply([{ op: OperationType.ADD, path: new Pointer(['logs', logs.length]), value: paths }]); }; const process = createProcess('test', [testCommandFactory('foo'), testCommandFactory('bar')], () => ({ diff --git a/tests/stores/unit/state/ImmutableState.ts b/tests/stores/unit/state/ImmutableState.ts index db152f2d6..acff94213 100644 --- a/tests/stores/unit/state/ImmutableState.ts +++ b/tests/stores/unit/state/ImmutableState.ts @@ -41,11 +41,11 @@ describe('state/ImmutableState', () => { it('value to array index path', () => { const state = new ImmutableState(); - const result = state.apply([ops.add({ path: '/test/0', state: null, value: null }, 'test')]); + const result = state.apply([ops.add({ path: ['test', 0], state: null, value: null }, 'test')]); assert.deepEqual(state.path('/test').value, ['test']); assert.deepEqual(result, [ - { op: OperationType.TEST, path: new Pointer('/test/0'), value: 'test' }, - { op: OperationType.REMOVE, path: new Pointer('/test/0') } + { op: OperationType.TEST, path: new Pointer(['test', 0]), value: 'test' }, + { op: OperationType.REMOVE, path: new Pointer(['test', 0]) } ]); }); }); @@ -171,12 +171,12 @@ describe('state/ImmutableState', () => { state.apply([ops.add({ path: '/foo/0/bar/baz/0/qux', state: null, value: null }, true)]); const result = state.apply([ops.test({ path: '/foo/0/bar/baz/0/qux', state: null, value: null }, true)]); assert.deepEqual(result, []); - assert.deepEqual(state.path('/foo').value, [{ bar: { baz: [{ qux: true }] } }]); + assert.deepEqual(state.path('/foo').value, { 0: { bar: { baz: { 0: { qux: true } } } } }); }); it('complex value', () => { const state = new ImmutableState(); - state.apply([ops.add({ path: '/foo/0/bar/baz/0/qux', state: null, value: null }, true)]); + state.apply([ops.add({ path: ['foo', 0, 'bar', 'baz', 0, 'qux'], state: null, value: null }, true)]); const result = state.apply([ ops.test({ path: '/foo', state: null, value: null }, [ { diff --git a/tests/stores/unit/state/Patch.ts b/tests/stores/unit/state/Patch.ts index b8383e24d..05a28bf98 100644 --- a/tests/stores/unit/state/Patch.ts +++ b/tests/stores/unit/state/Patch.ts @@ -154,13 +154,13 @@ describe('state/Patch', () => { }); it('array index path', () => { - const patch = new Patch(ops.remove({ path: '/test/1', state: null, value: null })); + const patch = new Patch(ops.remove({ path: ['test', 1], state: null, value: null })); const obj = { test: ['test', 'foo'] }; const result = patch.apply(obj); assert.notStrictEqual(result.object, obj); assert.deepEqual(result.object, { test: ['test'] }); assert.deepEqual(result.undoOperations, [ - { op: OperationType.ADD, path: new Pointer('/test/1'), value: 'foo' } + { op: OperationType.ADD, path: new Pointer(['test', 1]), value: 'foo' } ]); }); });