Extra info in JSONPatch mostly working

src/JSONNode.js

@@ -322,13 +322,13 @@ export default class JSONNode extends Component {
         },
         {
           text: 'Array',
-          className: 'jsoneditor-type-array' + (type == 'Array' ? ' jsoneditor-selected' : ''),
+          className: 'jsoneditor-type-Array' + (type == 'Array' ? ' jsoneditor-selected' : ''),
           title: TYPE_TITLES.array,
           click: () => events.onChangeType(path, 'Array')
         },
         {
           text: 'Object',
-          className: 'jsoneditor-type-object' + (type == 'Object' ? ' jsoneditor-selected' : ''),
+          className: 'jsoneditor-type-Object' + (type == 'Object' ? ' jsoneditor-selected' : ''),
           title: TYPE_TITLES.object,
           click: () => events.onChangeType(path, 'Object')
         },
@@ -389,13 +389,13 @@ export default class JSONNode extends Component {
           },
           {
             text: 'Array',
-            className: 'jsoneditor-type-array',
+            className: 'jsoneditor-type-Array',
             title: TYPE_TITLES.array,
             click: () => events.onInsert(path, 'Array')
           },
           {
             text: 'Object',
-            className: 'jsoneditor-type-object',
+            className: 'jsoneditor-type-Object',
             title: TYPE_TITLES.object,
             click: () => events.onInsert(path, 'Object')
           },
@@ -456,13 +456,13 @@ export default class JSONNode extends Component {
         },
         {
           text: 'Array',
-          className: 'jsoneditor-type-array',
+          className: 'jsoneditor-type-Array',
           title: TYPE_TITLES.array,
           click: () => events.onAppend(path, 'Array')
         },
         {
           text: 'Object',
-          className: 'jsoneditor-type-object',
+          className: 'jsoneditor-type-Object',
           title: TYPE_TITLES.object,
           click: () => events.onAppend(path, 'Object')
         },

src/TreeMode.js

@@ -5,7 +5,7 @@ import {
   expand, jsonToData, dataToJson, toDataPath, patchData, compileJSONPointer
 } from './jsonData'
 import {
-  duplicate, insert, append, changeType, changeValue, changeProperty, sort
+  duplicate, insert, append, remove, changeType, changeValue, changeProperty, sort
 } from './actions'
 import JSONNode from './JSONNode'
 
@@ -121,12 +121,7 @@ export default class TreeMode extends Component {
 
   /** @private */
   handleRemove = (path) => {
-    const patch = [{
+    this.handlePatch(remove(path))
-      op: 'remove',
-      path: compileJSONPointer(path)
-    }]
-
-    this.handlePatch(patch)
   }
 
   /** @private */

src/actions.js

@@ -1,9 +1,10 @@
-import { compileJSONPointer, toDataPath, dataToJson } from './jsonData'
+import { compileJSONPointer, toDataPath, dataToJson, findNextProp } from './jsonData'
 import { findUniqueName } from  './utils/stringUtils'
 import { getIn } from  './utils/immutabilityHelpers'
 import { isObject, stringConvert } from  './utils/typeUtils'
 import { compareAsc, compareDesc, strictShallowEqual } from './utils/arrayUtils'
 
+
 /**
  * Create a JSONPatch to change the value of a property or item
  * @param {JSONData} data
@@ -17,22 +18,14 @@ export function changeValue (data, path, value) {
   const dataPath = toDataPath(data, path)
   const oldDataValue = getIn(data, dataPath)
 
-  let patch = [{
+  return [{
     op: 'replace',
     path: compileJSONPointer(path),
-    value: value
+    value: value,
-  }]
+    jsoneditor: {
-
-  // when the old type is not something that can be detected from the
-  // value itself, store the type information
-  if(!isNativeType(oldDataValue.type)) {
-    // it's a string
-    patch[0].jsoneditor = {
       type: oldDataValue.type
     }
-  }
+  }]
-
-  return patch
 }
 
 /**
@@ -49,11 +42,6 @@ export function changeProperty (data, parentPath, oldProp, newProp) {
   const dataPath = toDataPath(data, parentPath)
   const parent = getIn(data, dataPath)
 
-  // find property after this one
-  const index = parent.props.findIndex(p => p.name === oldProp)
-  const next = parent.props[index + 1]
-  const nextProp = next && next.name
-
   // prevent duplicate property names
   const uniqueNewProp = findUniqueName(newProp, parent.props.map(p => p.name))
 
@@ -62,7 +50,7 @@ export function changeProperty (data, parentPath, oldProp, newProp) {
     from: compileJSONPointer(parentPath.concat(oldProp)),
     path: compileJSONPointer(parentPath.concat(uniqueNewProp)),
     jsoneditor: {
-      before: nextProp
+      before: findNextProp(parent, oldProp)
     }
   }]
 }
@@ -75,19 +63,19 @@ export function changeProperty (data, parentPath, oldProp, newProp) {
  * @return {Array}
  */
 export function changeType (data, path, type) {
-
   const dataPath = toDataPath(data, path)
-  const oldEntry = dataToJson(getIn(data, dataPath))
+  const oldValue = dataToJson(getIn(data, dataPath))
-  const newEntry = convertType(oldEntry, type)
+  const newValue = convertType(oldValue, type)
 
-  console.log('changeType', path, type, oldEntry, newEntry)
+  // console.log('changeType', path, type, oldValue, newValue)
 
   return [{
     op: 'replace',
     path: compileJSONPointer(path),
-    value: newEntry,
+    value: newValue,
-    jsoneditor: { type } // TODO: send type only in case of 'string'
+    jsoneditor: {
-    // TODO: send some information to ensure the correct order of fields?
+      type
+    }
   }]
 }
 
@@ -119,14 +107,16 @@ export function duplicate (data, path) {
     }]
   }
   else { // object.type === 'Object'
-    const afterProp = path[path.length - 1]
+    const prop = path[path.length - 1]
-    const newProp = findUniqueName(afterProp, parent.props.map(p => p.name))
+    const newProp = findUniqueName(prop, parent.props.map(p => p.name))
 
     return [{
       op: 'copy',
       from: compileJSONPointer(path),
       path: compileJSONPointer(parentPath.concat(newProp)),
-      jsoneditor: { afterProp }
+      jsoneditor: {
+        before: findNextProp(parent, prop)
+      }
     }]
   }
 }
@@ -156,18 +146,24 @@ export function insert (data, path, type) {
     return [{
       op: 'add',
       path: compileJSONPointer(parentPath.concat(index + '')),
-      value
+      value,
+      jsoneditor: {
+        type
+      }
     }]
   }
   else { // object.type === 'Object'
-    const afterProp = path[path.length - 1]
+    const prop = path[path.length - 1]
     const newProp = findUniqueName('', parent.props.map(p => p.name))
 
     return [{
       op: 'add',
       path: compileJSONPointer(parentPath.concat(newProp)),
       value,
-      jsoneditor: { afterProp }
+      jsoneditor: {
+        type,
+        before: findNextProp(parent, prop)
+      }
     }]
   }
 }
@@ -195,7 +191,10 @@ export function append (data, parentPath, type) {
     return [{
       op: 'add',
       path: compileJSONPointer(parentPath.concat('-')),
-      value
+      value,
+      jsoneditor: {
+        type
+      }
     }]
   }
   else { // object.type === 'Object'
@@ -204,11 +203,25 @@ export function append (data, parentPath, type) {
     return [{
       op: 'add',
       path: compileJSONPointer(parentPath.concat(newProp)),
-      value
+      value,
+      jsoneditor: {
+        type
+      }
     }]
   }
 }
 
+/**
+ * Create a JSONPatch for a remove action
+ * @param {Path} path
+ */
+export function remove (path) {
+  return [{
+    op: 'remove',
+    path: compileJSONPointer(path)
+  }]
+}
+
 /**
  * Create a JSONPatch to order the items of an array or the properties of an object in ascending
  * or descending order
@@ -338,13 +351,3 @@ export function convertType (value, type) {
 
   throw new Error(`Unknown type '${type}'`)
 }
-
-/**
- * Test whether a type is a native JSON type:
- * Native types are: Array, Object, or value
- * @param {JSONDataType} type
- * @return {boolean}
- */
-export function isNativeType (type) {
-  return type === 'Object' || type === 'Array' || type === 'value'
-}

src/jsonData.js

@@ -7,12 +7,13 @@ import { setIn, updateIn, getIn, deleteIn } from './utils/immutabilityHelpers'
 import { isObject } from  './utils/typeUtils'
 import isEqual from 'lodash/isEqual'
 
-// TODO: rewrite the functions into jsonpatch functions, including a function `patch`
+const expandAll = function (path) {
-
+  return true
-const expandNever = function (path) {
-  return false
 }
 
+// TODO: double check whether all patch functions handle each of the
+// extra properties in .jsoneditor: `before`, `type`, `order`
+
 /**
  * Convert a JSON object into the internally used data model
  * @param {Path} path
@@ -20,7 +21,7 @@ const expandNever = function (path) {
  * @param {function(path: Path)} expand
  * @return {JSONData}
  */
-// TODO: change signature to jsonToData(json, expand=(path) => false, path=[])
+// TODO: change signature to jsonToData(json [, expand=(path) => false [, path=[]]])
 export function jsonToData (path, json, expand) {
   if (Array.isArray(json)) {
     return {
@@ -107,20 +108,28 @@ export function toDataPath (data, path) {
  * @return {{data: JSONData, revert: Array.<Object>, error: Error | null}}
  */
 export function patchData (data, patch) {
-  const expand = expandNever   // TODO: customizable expand function
+  const expand = expandAll   // TODO: customizable expand function
 
   try {
     let updatedData = data
     let revert = []
 
     patch.forEach(function (action) {
+      const options = action.jsoneditor
+
       switch (action.op) {
         case 'add': {
           const path = parseJSONPointer(action.path)
-          const value = jsonToData(path, action.value, expand)
+          const newValue = jsonToData(path, action.value, expand)
-          const afterProp = getIn(action, ['jsoneditor', 'afterProp'])
 
-          const result = add(updatedData, action.path, value, afterProp)
+          // TODO: move setting type to jsonToData
+          if (options && options.type) {
+            // insert with type 'string' or 'value'
+            newValue.type = options.type
+          }
+          // TODO: handle options.order
+
+          const result = add(updatedData, action.path, newValue, options)
           updatedData = result.data
           revert.unshift(result.revert)
 
@@ -140,10 +149,11 @@ export function patchData (data, patch) {
           let newValue = jsonToData(path, action.value, expand)
 
           // TODO: move setting type to jsonToData
-          if (action.jsoneditor && action.jsoneditor.type) {
+          if (options && options.type) {
             // insert with type 'string' or 'value'
-            newValue.type = action.jsoneditor.type
+            newValue.type = options.type
           }
+          // TODO: handle options.order
 
           const result = replace(updatedData, path, newValue)
           updatedData = result.data
@@ -153,8 +163,7 @@ export function patchData (data, patch) {
         }
 
         case 'copy': {
-          const afterProp = getIn(action, ['jsoneditor', 'afterProp'])
+          const result = copy(updatedData, action.path, action.from, options)
-          const result = copy(updatedData, action.path, action.from, afterProp)
           updatedData = result.data
           revert.unshift(result.revert)
 
@@ -162,8 +171,7 @@ export function patchData (data, patch) {
         }
 
         case 'move': {
-          const afterProp = getIn(action, ['jsoneditor', 'afterProp'])
+          const result = move(updatedData, action.path, action.from, options)
-          const result = move(updatedData, action.path, action.from, afterProp)
           updatedData = result.data
           revert = result.revert.concat(revert)
 
@@ -205,11 +213,15 @@ export function replace (data, path, value) {
   const oldValue = dataToJson(getIn(data, dataPath))
 
   return {
+    // FIXME: keep the expanded state where possible
     data: setIn(data, dataPath, value),
     revert: {
       op: 'replace',
       path: compileJSONPointer(path),
-      value: oldValue
+      value: oldValue,
+      jsoneditor: {
+        type: oldValue.type
+      }
     }
   }
 }
@@ -222,34 +234,44 @@ export function replace (data, path, value) {
  */
 export function remove (data, path) {
   // console.log('remove', path)
-  const _path = parseJSONPointer(path)
+  const pathArray = parseJSONPointer(path)
 
-  const parentPath = _path.slice(0, _path.length - 1)
+  const parentPath = pathArray.slice(0, pathArray.length - 1)
   const parent = getIn(data, toDataPath(data, parentPath))
-  const dataValue = getIn(data, toDataPath(data, _path))
+  const dataValue = getIn(data, toDataPath(data, pathArray))
   const value = dataToJson(dataValue)
 
-  // extra information attached to the patch
-  const jsoneditor = {
-    type: dataValue.type
-    // FIXME: store before
-  }
-
   if (parent.type === 'Array') {
-    const dataPath = toDataPath(data, _path)
+    const dataPath = toDataPath(data, pathArray)
 
     return {
       data: deleteIn(data, dataPath),
-      revert: {op: 'add', path, value, jsoneditor}
+      revert: {
+        op: 'add',
+        path,
+        value,
+        jsoneditor: {
+          type: dataValue.type
+        }
+      }
     }
   }
   else { // object.type === 'Object'
-    const dataPath = toDataPath(data, _path)
+    const dataPath = toDataPath(data, pathArray)
+    const prop = pathArray[pathArray.length - 1]
 
     dataPath.pop()  // remove the 'value' property, we want to remove the whole object property
     return {
       data: deleteIn(data, dataPath),
-      revert: {op: 'add', path, value, jsoneditor}
+      revert: {
+        op: 'add',
+        path,
+        value,
+        jsoneditor: {
+          type: dataValue.type,
+          before: findNextProp(parent, prop)
+        }
+      }
     }
   }
 }
@@ -258,19 +280,17 @@ export function remove (data, path) {
  * @param {JSONData} data
  * @param {string} path
  * @param {JSONData} value
- * @param {string} [afterProp]   In case of an object, the property
+ * @param {{before?: string}} [options]
- *                               after which this new property must be added
- *                               can be specified
  * @return {{data: JSONData, revert: Object}}
  * @private
  */
-export function add (data, path, value, afterProp) {
+export function add (data, path, value, options) {
-  const _path = parseJSONPointer(path)
+  const pathArray = parseJSONPointer(path)
 
-  const parentPath = _path.slice(0, _path.length - 1)
+  const parentPath = pathArray.slice(0, pathArray.length - 1)
   const dataPath = toDataPath(data, parentPath)
   const parent = getIn(data, dataPath)
-  const resolvedPath = resolvePathIndex(data, _path)
+  const resolvedPath = resolvePathIndex(data, pathArray)
   const prop = resolvedPath[resolvedPath.length - 1]
 
   // FIXME: should not be needed to do try/catch. Create a function exists(data, path), or rewrite toDataPath such that you don't need to pass data
@@ -297,25 +317,41 @@ export function add (data, path, value, afterProp) {
     updatedData = updateIn(data, dataPath.concat('props'), (props) => {
       const newProp = { name: prop, value }
 
-      if (afterProp === undefined) {
+      if (!options || typeof options.before !== 'string') {
         // append
         return props.concat(newProp)
       }
       else {
         // insert after prop
         const updatedProps = props.slice(0)
-        const index = props.findIndex(p => p.name === afterProp)
+        const index = props.findIndex(p => p.name === options.before)
-        updatedProps.splice(index + 1, 0, newProp)
+        updatedProps.splice(index, 0, newProp)
         return updatedProps
       }
     })
   }
 
-  return {
+  if (parent.type === 'Object' && oldValue !== undefined) {
-    data: updatedData,
+    return {
-    revert: (parent.type === 'Object' && oldValue !== undefined)
+      data: updatedData,
-        ? {op: 'replace', path: compileJSONPointer(resolvedPath), value: dataToJson(oldValue)}
+      revert: {
-        : {op: 'remove', path: compileJSONPointer(resolvedPath)}
+        op: 'replace',
+        path: compileJSONPointer(resolvedPath),
+        value: dataToJson(oldValue),
+        jsoneditor: {
+          type: oldValue.type
+        }
+      }
+    }
+  }
+  else {
+    return {
+      data: updatedData,
+      revert: {
+        op: 'remove',
+        path: compileJSONPointer(resolvedPath)
+      }
+    }
   }
 }
 
@@ -324,16 +360,14 @@ export function add (data, path, value, afterProp) {
  * @param {JSONData} data
  * @param {string} path
  * @param {string} from
- * @param {string} [afterProp]   In case of an object, the property
+ * @param {{before?: string}} [options]
- *                               after which this new property must be added
- *                               can be specified
  * @return {{data: JSONData, revert: Object}}
  * @private
  */
-export function copy (data, path, from, afterProp) {
+export function copy (data, path, from, options) {
   const value = getIn(data, toDataPath(data, parseJSONPointer(from)))
 
-  return add(data, path, value, afterProp)
+  return add(data, path, value, options)
 }
 
 /**
@@ -341,40 +375,20 @@ export function copy (data, path, from, afterProp) {
  * @param {JSONData} data
  * @param {string} path
  * @param {string} from
- * @param {string} [afterProp]   In case of an object, the property
+ * @param {{before?: string}} [options]
- *                               after which this new property must be added
- *                               can be specified
  * @return {{data: JSONData, revert: Object}}
  * @private
  */
-export function move (data, path, from, afterProp) {
+export function move (data, path, from, options) {
   if (path !== from) {
     const value = getIn(data, toDataPath(data, parseJSONPointer(from)))
 
     const result1 = remove(data, from)
-    let updatedData = result1.data
+    const result2 = add(result1.data, path, value, options)
 
-    const result2 = add(updatedData, path, value, afterProp)
+    return {
-    updatedData = result2.data
+      data: result2.data,
-
+      revert: result1.revert.concat(result2.revert)
-    // FIXME: the revert action should store afterProp
-
-    if (result2.revert.op === 'replace') {
-      return {
-        data: updatedData,
-        revert: [
-          {op: 'move', from: path, path: from},
-          {op: 'add', path, value: result2.revert.value}
-        ]
-      }
-    }
-    else { // result2.revert.op === 'remove'
-      return {
-        data: updatedData,
-        revert: [
-          {op: 'move', from: path, path: from}
-        ]
-      }
     }
   }
   else {
@@ -394,8 +408,8 @@ export function move (data, path, from, afterProp) {
  * @param {*} value
  */
 export function test (data, path, value) {
-  const _path = parseJSONPointer(path)
+  const pathArray = parseJSONPointer(path)
-  const actualValue = getIn(data, toDataPath(data, _path))
+  const actualValue = getIn(data, toDataPath(data, pathArray))
 
   if (value === undefined) {
     throw new Error('Test failed, no value provided')
@@ -539,6 +553,20 @@ export function resolvePathIndex (data, path) {
   return path
 }
 
+/**
+ * Find the property after provided property
+ * @param {JSONData} parent
+ * @param {string} prop
+ * @return {string | null} Returns the name of the next property,
+ *                         or null if there is none
+ */
+export function findNextProp (parent, prop) {
+  const index = parent.props.findIndex(p => p.name === prop)
+  const next = parent.props[index + 1]
+
+  return next && next.name || null
+}
+
 /**
  * Parse a JSON Pointer
  * WARNING: this is not a complete string implementation

src/jsoneditor.less

@@ -464,20 +464,20 @@ button.jsoneditor-type-value.jsoneditor-selected span.jsoneditor-icon {
   background-position: -120px 0;
 }
 
-button.jsoneditor-type-object span.jsoneditor-icon {
+button.jsoneditor-type-Object span.jsoneditor-icon {
   background-position: -72px -24px;
 }
-button.jsoneditor-type-object:hover span.jsoneditor-icon,
+button.jsoneditor-type-Object:hover span.jsoneditor-icon,
-button.jsoneditor-type-object:focus span.jsoneditor-icon,
+button.jsoneditor-type-Object:focus span.jsoneditor-icon,
-button.jsoneditor-type-object.jsoneditor-selected span.jsoneditor-icon {
+button.jsoneditor-type-Object.jsoneditor-selected span.jsoneditor-icon {
   background-position: -72px 0;
 }
 
-button.jsoneditor-type-array span.jsoneditor-icon {
+button.jsoneditor-type-Array span.jsoneditor-icon {
   background-position: -96px -24px;
 }
-button.jsoneditor-type-array:hover span.jsoneditor-icon,
+button.jsoneditor-type-Array:hover span.jsoneditor-icon,
-button.jsoneditor-type-array:focus span.jsoneditor-icon,
+button.jsoneditor-type-Array:focus span.jsoneditor-icon,
-button.jsoneditor-type-array.jsoneditor-selected span.jsoneditor-icon {
+button.jsoneditor-type-Array.jsoneditor-selected span.jsoneditor-icon {
   background-position: -96px 0;
 }