Merge pull request #230 from blueflag/feature/merge-mode

Refactor lastOriginId and add mergeMode

packages/dataparcels-docs/src/layout/index.scss

@@ -75,6 +75,9 @@
         text-shadow: -6px 6px 0px lighten(color('primary'), 10),
             -12px 12px 0px lighten(color('primary'), 30)
     }
+    &-code {
+        overflow: auto;
+    }
 }
 @include DcmeTypography {
     input {

packages/dataparcels-docs/src/pages/index.mdx

@@ -69,9 +69,6 @@ I hope this library helps solve some front-end problems for you.
 
 ### Roadmap
 
-- Add **revertable changes**. The `useParcelState.onChange` callback should be able to return promises, and this can then allow failed `onChange` calls to reinstate unsaved changes in a lower Parcel buffer. This is a crucial feature that will allow for forms to rollback when requests fail.
-- Add **data synchronisation docs**, including data sync strategies strategies with examples.
-- Add **merge mode** to control how downward changes are accepted into `ParcelBoundary` components. This is required for rekey.
 - Add **rekey**, which enables changes via props to be merged into buffered changes (i.e. unsaved changes). This will allow multiple editors to alter the same piece of data simultaneously without overwriting. The ability to rebase unsaved changes onto updated data already exists, but rekey is required to make sense of incoming changes via props.
 - Add **cache**, an option in `useParcelBuffer` to save, reload and clear cached data. This can be used with `localStorage` or similar external storage mechanisms to retain and restore unsaved changes.
 - Add **production builds**, a proper build process that doesn't rely on minification and dead code elimination being carried out by the containing project's build process. This step will finally allow proper optimisations to reduce bundle size.

packages/dataparcels-docs/yalc.lock

@@ -2,7 +2,7 @@
   "version": "v1",
   "packages": {
     "react-dataparcels": {
-      "signature": "399c419dd23445b15c9a03b7fcae9341",
+      "signature": "7640a04beb7f2428153fe8382775b5e7",
       "file": true,
       "replaced": "^0.21.0"
     },

packages/dataparcels/src/errors/Errors.js

@@ -5,4 +5,3 @@ export const ReducerInvalidActionError = (actionType: string) => new Error(`"${a
 export const ReducerInvalidStepError = (stepType: string) => new Error(`"${stepType}" is not a valid action step type`);
 export const ChangeRequestNoPrevDataError = () =>  new Error(`ChangeRequest data cannot be accessed before setting changeRequest.prevData`);
 export const ShapeUpdaterNonShapeChildError = () =>  new Error(`Every child value on a collection returned from a shape updater must be a ParcelShape`);
-export const ChangeAndReturnNotCalledError = () =>  new Error(`_changeAndReturn unchanged`);

packages/dataparcels/src/parcel/Parcel.js

@@ -17,7 +17,6 @@ import type {ParentType} from '../types/Types';
 
 import Types from '../types/Types';
 import {ReadOnlyError} from '../errors/Errors';
-import {ChangeAndReturnNotCalledError} from '../errors/Errors';
 
 import ParcelGetMethods from './methods/ParcelGetMethods';
 import ParcelChangeMethods from './methods/ParcelChangeMethods';
@@ -41,7 +40,7 @@ import overload from 'unmutable/lib/util/overload';
 const DEFAULT_CONFIG_INTERNAL = () => ({
     child: undefined,
     dispatchId: '',
-    lastOriginId: '',
+    frameMeta: {},
     meta: {},
     id: new ParcelId(),
     parent: {
@@ -67,15 +66,15 @@ export default class Parcel {
         let {
             child,
             dispatchId,
-            lastOriginId,
+            frameMeta,
             meta,
             id,
             parent,
             registry,
             updateChangeRequestOnDispatch
         } = _configInternal || DEFAULT_CONFIG_INTERNAL();
 
-        this._lastOriginId = lastOriginId;
+        this._frameMeta = frameMeta;
         this._onHandleChange = handleChange;
         this._updateChangeRequestOnDispatch = updateChangeRequestOnDispatch;
 
@@ -124,15 +123,15 @@ export default class Parcel {
     //
 
     // from constructor
-    _childParcelCache: { [key: string]: Parcel } = {};
+    _childParcelCache: {[key: string]: Parcel} = {};
     _dispatchId: string;
     _id: ParcelId;
     _isChild: boolean;
     _isElement: boolean;
     _isIndexed: boolean;
     _isParent: boolean;
-    _lastOriginId: string;
-    _methods: { [key: string]: * };
+    _frameMeta: {[key: string]: any};
+    _methods: {[key: string]: any};
     _onHandleChange: ?Function;
     _parcelData: ParcelData;
     _parent: ParcelParent;
@@ -148,7 +147,7 @@ export default class Parcel {
             dispatchId = this._id.id(),
             handleChange,
             id = this._id,
-            lastOriginId = this._lastOriginId,
+            frameMeta = this._frameMeta,
             parcelData = this._parcelData,
             parent = this._parent,
             registry = this._registry,
@@ -169,7 +168,7 @@ export default class Parcel {
             {
                 child,
                 dispatchId,
-                lastOriginId,
+                frameMeta,
                 meta,
                 id,
                 parent,
@@ -188,24 +187,21 @@ export default class Parcel {
         }
     };
 
-    _changeAndReturn = (changeCatcher: (parcel: Parcel) => void): [Parcel, ChangeRequest] => {
+    _changeAndReturn = (changeCatcher: (parcel: Parcel) => void): [Parcel, ?ChangeRequest] => {
         let result;
-        let {_onHandleChange, _lastOriginId} = this;
+        let {_onHandleChange} = this;
 
         // swap out the parcels real _onHandleChange with a spy
         this._onHandleChange = (parcel, changeRequest) => {
-            // _changeAndReturn should not alter _lastOriginId
-            // as it's never triggered by a user action
-            // so revert to the current parcel's _lastOriginId
             parcel._onHandleChange = _onHandleChange;
-            parcel._lastOriginId = _lastOriginId;
+            parcel._frameMeta = this._frameMeta;
             result = [parcel, changeRequest];
         };
 
         changeCatcher(this);
         this._onHandleChange = _onHandleChange;
         if(!result) {
-            throw ChangeAndReturnNotCalledError();
+            return [this, undefined];
         }
         return result;
     };

packages/dataparcels/src/parcel/__test__/Parcel-test.js

@@ -36,7 +36,6 @@ test('Parcel._changeAndReturn() should call action and return Parcel', () => {
         },
         handleChange
     });
-    parcel._lastOriginId = "foo";
 
     let [newParcel] = parcel._changeAndReturn((parcel) => {
         parcel.get('abc').onChange(789);
@@ -59,12 +58,11 @@ test('Parcel._changeAndReturn() should call action and return Parcel', () => {
     newParcel.get('abc').onChange(100);
     expect(handleChange).toHaveBeenCalledTimes(2);
 
-    // _changeAndReturn should not affect parcel._lastOriginId as it is an internal function
-    // that never corresponds to actions triggered by user input
-    expect(newParcel._lastOriginId).toBe("foo");
+    // _frameMeta should be passed through
+    expect(parcel._frameMeta).toBe(newParcel._frameMeta);
 });
 
-test('Parcel._changeAndReturn() should throw error if no changes are made', () => {
+test('Parcel._changeAndReturn() should return [parcel, undefined] if no changes are made', () => {
     let handleChange = jest.fn();
 
     let parcel = new Parcel({
@@ -75,7 +73,9 @@ test('Parcel._changeAndReturn() should throw error if no changes are made', () =
         handleChange
     });
 
-    expect(() => parcel._changeAndReturn((parcel) => {})).toThrow("_changeAndReturn unchanged");
+    let result = parcel._changeAndReturn(() => {});
+
+    expect(result).toEqual([parcel, undefined]);
 });
 
 test('Parcel types should correctly identify primitive values', () => {
@@ -267,3 +267,28 @@ test('Correct methods are created for array element values', () => {
     expect(() => new Parcel(data).get(0).swapNext()).not.toThrow();
 });
 
+test('Frame meta should be passed down to child parcels', () => {
+    let parcel = new Parcel({
+        value: [[123]]
+    });
+
+    parcel._frameMeta.foo = 123;
+
+    expect(parcel.get(0)._frameMeta.foo).toBe(123);
+    expect(parcel.get(0).get(0)._frameMeta.foo).toBe(123);
+});
+
+test('Frame meta should not persist after change', () => {
+    let handleChange = jest.fn();
+
+    let parcel = new Parcel({
+        value: 123,
+        handleChange
+    });
+
+    parcel._frameMeta.foo = "bar";
+    parcel.set(456);
+
+    expect(handleChange.mock.calls[0][0]._frameMeta).toEqual({});
+});
+

packages/dataparcels/src/parcel/methods/ParcelChangeMethods.js

@@ -29,6 +29,12 @@ export default (_this: Parcel) => ({
             changeRequest._originPath = _this.path;
         }
 
+        // clear changeRequest's cache
+        changeRequest = changeRequest._create({
+            prevData: undefined,
+            nextData: undefined
+        });
+
         if(process.env.NODE_ENV !== 'production' && _this._log) {
             console.log(`Parcel: "${_this._logName}" data up:`); // eslint-disable-line
             console.log(changeRequest.toJS()); // eslint-disable-line
@@ -47,7 +53,7 @@ export default (_this: Parcel) => ({
             let parcelWithChangedData = _this._create({
                 handleChange: _onHandleChange,
                 parcelData,
-                lastOriginId: changeRequest.originId
+                frameMeta: {}
             });
 
             _onHandleChange(parcelWithChangedData, changeRequestWithBase);

packages/dataparcels/src/types/Types.js

@@ -31,7 +31,7 @@ export type ParcelConfigInternal = {
     child: *,
     dispatchId: string,
     id: ParcelId,
-    lastOriginId: string,
+    frameMeta: {[key: string]: any},
     meta: ParcelMeta,
     parent: ParcelParent,
     registry: ParcelRegistry,
@@ -40,7 +40,7 @@ export type ParcelConfigInternal = {
 
 export type ParcelCreateConfigType = {
     dispatchId?: string,
-    lastOriginId?: string,
+    frameMeta?: {[key: string]: any},
     id?: ParcelId,
     handleChange?: Function,
     parcelData?: ParcelData,

packages/react-dataparcels/src/ParcelBoundaryDeprecated.jsx

@@ -125,7 +125,7 @@ export default class ParcelBoundary extends React.Component<Props, State> { /* e
             let newData = parcel.data;
 
             if(keepValue) {
-                let changedBySelf = parcel._lastOriginId.startsWith(parcel.id);
+                let changedBySelf = parcel._frameMeta.lastOriginId.startsWith(parcel.id);
                 if(changedBySelf) {
                     newState.lastValueFromSelf = parcel.value;
                 }

packages/react-dataparcels/src/ParcelHoc.jsx

@@ -138,6 +138,10 @@ export default (config: ParcelHocConfig): Function => {
         }
 
         handleChange = (parcel: Parcel, changeRequest: ChangeRequest) => {
+            parcel._frameMeta = {
+                lastOriginId: changeRequest.originId
+            };
+
             this.setState({parcel});
             if(process.env.NODE_ENV !== 'production' && debugParcel) {
                 console.log(`ParcelHoc: Parcel changed:`); // eslint-disable-line

packages/react-dataparcels/src/__test__/ParcelBoundaryDeprecated-test.js

@@ -467,6 +467,9 @@ test('ParcelBoundary should not update value from props for updates caused by th
     childParcel.onChange(456);
 
     let newParcel = handleChange.mock.calls[0][0];
+    newParcel._frameMeta = {
+        lastOriginId: handleChange.mock.calls[0][1].originId
+    };
 
     // verify that the current value of the parcel has been updated
     expect(newParcel.value).toBe(457);
@@ -483,6 +486,9 @@ test('ParcelBoundary should not update value from props for updates caused by th
     // make a change externally and ensure that the value in the boundary does update
     newParcel.set(789);
     let newParcel2 = handleChange.mock.calls[1][0];
+    newParcel2._frameMeta = {
+        lastOriginId: handleChange.mock.calls[1][1].originId
+    };
     wrapper.setProps({
         parcel: withModify(newParcel2)
     });
@@ -510,6 +516,9 @@ test('ParcelBoundary should update meta from props for updates caused by themsel
     childParcel.onChange(456);
 
     let newParcel = handleChange.mock.calls[0][0];
+    newParcel._frameMeta = {
+        lastOriginId: handleChange.mock.calls[0][1].originId
+    };
 
     // make a change that keepValue will prevent from altering its value
     wrapper.setProps({
@@ -522,6 +531,9 @@ test('ParcelBoundary should update meta from props for updates caused by themsel
         abc: 789
     });
     let newParcel2 = handleChange.mock.calls[1][0];
+    newParcel2._frameMeta = {
+        lastOriginId: handleChange.mock.calls[1][1].originId
+    };
     wrapper.setProps({
         parcel: withModify(newParcel2)
     });

packages/react-dataparcels/src/__test__/useParcelBuffer-test.js

@@ -72,14 +72,18 @@ describe('useParcelBuffer should use config.parcel', () => {
         expect(result.current[0]).toBe(firstResult);
     });
 
-    it('should pass new inner parcel if outer parcel is different', () => {
+    it('should pass new inner parcel and clear buffer contents if outer parcel is different', () => {
 
         let parcel = new Parcel({
             value: 123
         });
 
         let {result, rerender} = renderHookWithProps({parcel}, ({parcel}) => useParcelBuffer({parcel}));
 
+        act(() => {
+            result.current[0].set(124);
+        });
+
         act(() => {
             rerender({
                 parcel: new Parcel({
@@ -88,7 +92,57 @@ describe('useParcelBuffer should use config.parcel', () => {
             });
         });
 
+        // inner parcel should have outer parcels value
         expect(result.current[0].value).toEqual(456);
+        // buffer should be cleared
+        expect(result.current[1].actions.length).toBe(0);
+    });
+
+    it('should keep inner parcel and buffer contents if outer parcel is different and mergeMode is "rebase"', () => {
+
+        let parcel = new Parcel({
+            value: {
+                abc: 100,
+                def: 100
+            }
+        });
+
+        let {result, rerender} = renderHookWithProps({parcel}, ({parcel}) => useParcelBuffer({parcel}));
+
+        act(() => {
+            result.current[0].set('abc', 400);
+        });
+
+        // confirm that set() has worked
+        expect(result.current[0].value).toEqual({
+            abc: 400,
+            def: 100
+        });
+
+        act(() => {
+            let parcel = new Parcel({
+                value: {
+                    abc: 200,
+                    def: 200
+                }
+            });
+
+            parcel._frameMeta.mergeMode = "rebase";
+
+            rerender({
+                parcel
+            });
+        });
+
+        // actions should be rebased onto new parcel from props
+        // and inner parcel should contain the resulting data
+        expect(result.current[0].value).toEqual({
+            abc: 400,
+            def: 200
+        });
+
+        // buffer should remain
+        expect(result.current[1].actions.length).toBe(1);
     });
 
 });
@@ -272,7 +326,7 @@ describe('useParcelBuffer should use config.debounce', () => {
         });
 
         expect(handleChange.mock.calls[0][0].value).toEqual(["A", "B"]);
-        expect(handleChange.mock.calls[1][0].value).toEqual(["A", "B", "C"]);
+        expect(handleChange.mock.calls[1][0].value).toEqual(["C"]);
     });
 
     it('should flush debounced changes if config.debounce is removed', () => {
@@ -440,6 +494,9 @@ describe('useParcelBuffer should use config.keepValue', () => {
         });
 
         let newParcel = handleChange.mock.calls[0][0];
+        newParcel._frameMeta = {
+            lastOriginId: handleChange.mock.calls[0][1].originId
+        };
 
         expect(newParcel.value).toEqual({
             abc: NaN