Merge pull request #79 from andrewcourtice/fix/history-array-mutable

Fix mutable array methods in history extension

extensions/history/src/constants.ts

@@ -1,3 +1,12 @@
+import {
+    NOTHING,
+} from '@harlem/extension-trace';
+
+import {
+    typeIsAny,
+    typeIsArray,
+} from '@harlem/utilities';
+
 import type {
     ChangeCommands,
     ChangeType,
@@ -18,7 +27,15 @@ export const CHANGE_MAP: Record<ChangeType, Partial<ChangeCommands>> = {
         deleteProperty: (target, prop) => delete target[prop],
     },
     undo: {
-        set: (target, prop, newValue, oldValue) => target[prop] = oldValue,
+        set: (target, prop, newValue, oldValue) => {
+            if (oldValue !== NOTHING) {
+                return target[prop] = oldValue;
+            }
+
+            (typeIsArray(target) && typeIsAny(prop, ['number', 'string']))
+                ? target.splice(+prop, 1)
+                : delete target[prop];
+        },
         deleteProperty: (target, prop, newValue, oldValue) => target[prop] = oldValue,
     },
 };

extensions/history/src/index.ts

@@ -208,7 +208,7 @@ export default function historyExtension<TState extends BaseState>(options?: Par
                 trigger(EVENTS.change.after);
             }
 
-            group.position = numberClamp(-1, group.history.length - 1, group.position + offset);
+            group.position = numberClamp(group.position + offset, -1, group.history.length - 1);
         }
 
         function undo(group: string = DEFAULT_GROUP_KEY) {

extensions/history/test/history.test.ts

@@ -17,12 +17,14 @@ import {
 
 describe('History Extension', () => {
 
+    const DUPLICATE_ROLE_MUTATION = 'duplicate-role';
+
     const getInstance = () => getStore({
         extensions: [
             historyExtension({
                 mutations: {
                     groups: {
-                        default: ['set-user-id'],
+                        default: ['set-user-id', DUPLICATE_ROLE_MUTATION],
                         userDetails: ['set-user-details'],
                     },
                 },
@@ -61,6 +63,28 @@ describe('History Extension', () => {
         expect(state.id).toBe(10);
     });
 
+    test('Handles mutable array methods', () => {
+        const {
+            store,
+        } = instance;
+
+        const {
+            state,
+            undo,
+            mutation,
+        } = store;
+
+        const duplicateRole = mutation(DUPLICATE_ROLE_MUTATION, ({ roles }, value: string) => {
+            roles.splice(roles.indexOf(value), 0, value);
+        });
+
+        expect(state.roles.length).toBe(2);
+        duplicateRole(state.roles[0]);
+        expect(state.roles.length).toBe(3);
+        undo();
+        expect(state.roles.length).toBe(2);
+    });
+
     test('Performs a grouped undo/redo operation', () => {
         const {
             store,

extensions/trace/src/constants.ts

@@ -3,6 +3,8 @@ import type {
     TraceGate,
 } from './types';
 
+export const NOTHING = Symbol('nothing');
+
 export const TAG_STYLE = {
     foreground: '#FFFFFF',
     background: '#6B7280',

extensions/trace/src/index.ts

@@ -2,6 +2,7 @@
 
 import {
     GATE_TAG_STYLE,
+    NOTHING,
     TAG_STYLE,
 } from './constants';
 
@@ -34,6 +35,7 @@ import type {
     TrackEventOptions,
 } from './types';
 
+export { NOTHING } from './constants';
 export * from './types';
 
 const GATE_MAP = {
@@ -50,15 +52,17 @@ const GATE_MAP = {
             paths: paths.concat(prop),
         });
     },
-    set: (callback, { paths }) => (target, prop, value) => {
-        defaultCallback(callback, 'set', paths, prop, target[prop], value);
-        target[prop] = value;
-        return true;
+    set: (callback, { paths }) => (target, prop, value, receiver) => {
+        const oldValue = prop in target
+            ? target[prop]
+            : NOTHING;
+
+        defaultCallback(callback, 'set', paths, prop, oldValue, value);
+        return Reflect.set(target, prop, value, receiver);
     },
     deleteProperty: (callback, { paths }) => (target, prop) => {
         defaultCallback(callback, 'deleteProperty', paths, prop, target[prop]);
-        delete target[prop];
-        return true;
+        return Reflect.deleteProperty(target, prop);
     },
 } as GateMap;
 

packages/testing/src/store.ts

@@ -24,6 +24,10 @@ const STATE = {
         lastName: '',
         age: 0,
     },
+    roles: [
+        'viewer',
+        'editor',
+    ],
 };
 
 export function jsonClone<TValue>(value: TValue): TValue {

packages/utilities/src/index.ts

@@ -15,6 +15,7 @@ export { default as objectToPath } from './object/to-path';
 export { default as objectTrace } from './object/trace';
 
 export { default as typeGetType } from './type/get-type';
+export { default as typeIsAny } from './type/is-any';
 export { default as typeIsArray } from './type/is-array';
 export { default as typeIsBoolean } from './type/is-boolean';
 export { default as typeIsFunction } from './type/is-function';

packages/utilities/src/number/clamp.ts

@@ -1,3 +1,3 @@
-export default function clamp(lower: number, upper: number, value: number) {
+export default function clamp(value: number, lower: number, upper: number) {
     return Math.max(lower, Math.min(upper, value));
 }

packages/utilities/src/type/is-any.ts

@@ -0,0 +1,10 @@
+import {
+    RuntimeType,
+    RuntimeTypeMap,
+} from '../types';
+
+import getType from './get-type';
+
+export default function typeIsAny<TType extends RuntimeType>(value: unknown, types: TType[]): value is RuntimeTypeMap[TType] {
+    return types.includes(getType(value) as TType);
+}

packages/utilities/src/types.ts

@@ -17,17 +17,20 @@ export interface Matchable {
     exclude: Matcher;
 }
 
-export type RuntimeType = 'boolean'
-| 'number'
-| 'string'
-| 'error'
-| 'date'
-| 'regexp'
-| 'function'
-| 'symbol'
-| 'array'
-| 'object'
-| 'map'
-| 'set'
-| 'null'
-| 'undefined';
+export type RuntimeType = keyof RuntimeTypeMap;
+export interface RuntimeTypeMap {
+    boolean: boolean;
+    number: number;
+    string: string;
+    error: Error;
+    date: Date;
+    regexp: RegExp;
+    function: Function;
+    symbol: symbol;
+    array: Array<unknown>;
+    object: object;
+    map: Map<unknown, unknown>;
+    set: Set<unknown>;
+    null: null;
+    undefined: undefined;
+}

packages/utilities/test/number/clamp.test.ts

@@ -0,0 +1,20 @@
+import clamp from '../../src/number/clamp';
+
+import {
+    describe,
+    expect,
+    test,
+} from 'vitest';
+
+describe('Utilities', () => {
+
+    describe('Number Clamp', () => {
+
+        test('Should clamp the given value between the specified lower/upper bounds', () => {
+            expect(clamp(25, 10, 20)).toBe(20);
+            expect(clamp(-50, 0, 100)).toBe(0);
+        });
+
+    });
+
+});

packages/utilities/test/type/is-any.test.ts

@@ -0,0 +1,21 @@
+import isAny from '../../src/type/is-any';
+
+import {
+    describe,
+    expect,
+    test,
+} from 'vitest';
+
+describe('Utilities', () => {
+
+    describe('Type Is Any', () => {
+
+        test('Should verify that a given value matches the specified types', () => {
+            expect(isAny(true, ['boolean', 'date'])).toBe(true);
+            expect(isAny(true, ['string', 'symbol'])).toBe(false);
+            expect(isAny('hello', ['date', 'object'])).toBe(false);
+        });
+
+    });
+
+});

packages/utilities/test/type/is-matchable.test.ts

@@ -0,0 +1,22 @@
+import isMatchable from '../../src/type/is-matchable';
+
+import {
+    describe,
+    expect,
+    test,
+} from 'vitest';
+
+describe('Utilities', () => {
+
+    describe('Type Is Matchable', () => {
+
+        test('Should verify that a given value is matchable', () => {
+            expect(isMatchable({
+                include: '*',
+                exclude: ['errors'],
+            })).toBe(true);
+        });
+
+    });
+
+});