derbyjs / racer

Realtime model synchronization engine for Node.js
1.18k stars 116 forks source link

If `cb` is undefined, `childModel.add` inputs for `segment` and `value` are incorrect #268

Closed hdavidzhu closed 2 months ago

hdavidzhu commented 4 years ago

https://github.com/derbyjs/racer/blob/292129c9086551a6415452bbc3ce5d368283ce8a/lib/Model/mutators.js#L202-L208

Example:

var $books = model.at('books');

// If cb === undefined, this would error
$books.add({title: 'Hello World'}, cb);
michael-brade commented 4 years ago

There is some missing information here: you are talking about passing a second argument explicitely, and that second argument's value is undefined. If you look at racer and derby, the code checks for the number of passed arguments all over the place, so if you really want to change this case, you should change it everywhere. Methods to look at are: Model.set, Model.add, Model.increment, Model.push, Model.unshift, Model.insert, Model.remove, Model.move, and many more.

However, if you think about it: Model.add could check if arguments[0] is an object, but Model.set for instance cannot. arguments[0] is allowed to be a string, so how would you know that ('hello', undefined) means setting hello to the current path and not setting undefined to the subpath hello?

That said, I don't think this is a bug or that it could be fixed consistently.

craigbeck commented 2 months ago

Thanks for your contributing your issue.

We have recently been actively updating the Derby and Racer packages, and both repos are now in Typescript and published with types on npm. As we have quite a few old issues open we have made the decision to close out of date issues.

If this issue still matters to you we encourage reproducing against the current versions of the repo and opening a new issue.