It doesn't work for simple types

pipex commented 1 year ago

Here is a simple replication, I would expect the result to be 1

> applyPatch = require('rfc6902').applyPatch
[Function: applyPatch]
> applyPatch(0, [ { op: 'replace', path: '', value: 1 } ])
[
  Error [MissingError]: Value required at path:
      at new MissingError (/Users/flalanne/Development/balena/mahler/parallel-tasks/node_modules/rfc6902/patch.js:25:28)
      at replace (/Users/flalanne/Development/balena/mahler/parallel-tasks/node_modules/rfc6902/patch.js:119:16)
      at Object.apply (/Users/flalanne/Development/balena/mahler/parallel-tasks/node_modules/rfc6902/patch.js:228:32)
      at /Users/flalanne/Development/balena/mahler/parallel-tasks/node_modules/rfc6902/index.js:24:60
      at Array.map (<anonymous>)
      at applyPatch (/Users/flalanne/Development/balena/mahler/parallel-tasks/node_modules/rfc6902/index.js:24:18)
      at REPL38:1:1
      at Script.runInThisContext (node:vm:129:12)
      at REPLServer.defaultEval (node:repl:572:29)
      at bound (node:domain:433:15) {
    path: ''
  }
]

chbrown commented 1 year ago

I'll admit it may not be the best API design, but it's small (two functions) and adequately (IMHO) documented: https://github.com/chbrown/rfc6902#api

You might reasonably expect the result of your call to be [null], which may well be more correct but is not more useful (though I'll grant that a more useful error message is well within reach 😉), since JavaScript cannot apply a change to a "simple" (primitive) type.

Even without explicitly learning concepts like pointers or "pass-by-reference", a novice JS programmer will quickly develop the intuition that if you have a function blackbox, and a var x = 123, there's no way to call blackbox(x, ...otherArgs) and end up with x being anything other than 123.

So IDK what you want... a new API? JavaScript to work differently?

If you must have a different API, it wouldn't be too hard to wrap this library's applyPatch with something like:

function applyPatchButFPStyle(object, patch) {
  var container = {root: object}
  var patchWithRootPrependedToAllPaths = ... // TODO
  var results = applyPatch(container, patchWithRootPrependedToAllPaths)
  if (results.any(result => result != null)) {
    throw results
  }
  return container.root
}
var thisIsZero = 0
var thisWillBeOne = applyPatchButFPStyle(thisIsZero, [{op: 'replace', path: '', value: 1}])

... though having to adjust all the paths is kind of annoying.

pipex commented 1 year ago

Thanks for the reply and the workaround. I opened up the issue as a quick report on some unexpected behavior when comparing with other json patch implementations, but you are right my report doesn’t make sense considering that applyPatch modifies the object in place. I’ll close the issue now.

chbrown / rfc6902

It doesn't work for simple types #92