Skip to content
This repository has been archived by the owner on Dec 20, 2023. It is now read-only.

"Can't use add op without position" adding properties to object #12

Open
withoutlogin opened this issue Feb 7, 2020 · 10 comments
Open

Comments

@withoutlogin
Copy link

Hi,

I think there's some kind of regression about following closed issue:
#6

With the latest npm version (1.0.0), I have following issue:

import * as jsonpatch from "fast-json-patch";
import toMongodb from "jsonpatch-to-mongodb";

const obj1 = { key: "value" };
const obj2 = { key: "value", newKey: "newValue" };
const patch = jsonpatch.compare(obj1, obj2); // [ { op: 'add', path: '/newKey', value: 'newValue' } ] 
toMongodb(patch); // throws: Unsupported Operation! can't use add op without position

As you can see patch is fully correct and should result with {$set: {newKey: "newValue"}}.

Thanks.

@withoutlogin withoutlogin changed the title "Can't use op without position" adding properties to object "Can't use add op without position" adding properties to object Feb 7, 2020
@mrcranky
Copy link

mrcranky commented Mar 9, 2020

It looks like the new code assumes that adds are only ever adding values to arrays maybe?

Given how the spec is written, I'm not sure it's possible to write a complete Mongo update operation from just the patch document alone? Don't you need a copy of the target document so that you can test for whether or not the path targets properties which already exist, and whether or not they're objects / arrays?

@mrcranky
Copy link

Actually, for this case at least, it seems like the spec can be handled without knowledge of the target object. We're dealing with the second and third cases from https://tools.ietf.org/html/rfc6902#section-4.1:
o If the target location specifies an array index, a new value is
inserted into the array at the specified index.
o If the target location specifies an object member that does not
already exist, a new member is added to the object.
o If the target location specifies an object member that does exist,
that member's value is replaced.

Case 1 has been handled in the code, cases 2 and 3 both happen to map to $set, and that will have the desired effect regardless of whether or not the target location already exists. I'll see about making a pull request to cover this case.

@ddimitrioglo
Copy link

Any updates on this?
Facing the same issue.

@mrcranky
Copy link

When the PR wasn't accepted, I ended up going ahead and spending a day or so rolling our own module to do this, along the lines of my previous reply above; i.e. taking the existing document as an additional parameter so that operations can be applied with knowledge of the previous document. Sadly it is not open sourced, though I will ask if our company is okay with publishing it.

@ddimitrioglo
Copy link

@mrcranky that would be great!
Looking forward! 🤞 😉

@ratio91
Copy link

ratio91 commented Feb 17, 2021

I have the same issue. would appreciate if you could share your solution @mrcranky. thanks!

@gadicc
Copy link

gadicc commented Jun 9, 2022

annual ping for @mrcranky to see if you were able to publish this 🙏 😁

@mrcranky
Copy link

mrcranky commented May 3, 2023

annual ping for @mrcranky to see if you were able to publish this 🙏 😁

Okay, so, pre-empting the next annual ping, I have some positive news. I left my previous job, which never got around to giving the go-ahead, and so have had enough time to make https://www.npmjs.com/package/rfc6902-mongodb which is another attempt at the same functionality. It's my first npm module / github open source project, so if folks want to test it out, issues can be reported here and I'll see about fixing them.

@gadicc
Copy link

gadicc commented May 4, 2023

Ah amazing! Thanks so much, @mrcranky, and congrats on your first package! I'll check this out when I'm next working on the relevant package of mine... might be a while, but I've starred your repo in the meantime and thanks so much for reporting back and publishing! 🎉

@ddimitrioglo
Copy link

Thank you @mrcranky!
I'll definitely keep your new package in mind when I have some related work to do.

Regarding the first package - it's awesome!
Small note: do not include tests into the package, and think of using typescript, which would make the maintenance work much easier.
Anyway - good luck!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants