.eql explodes with thinky

[kaievns]

hey andri, i thought i'd report this one. if i have two instances of a thinky model and then try to compare them against each other with .eql like so:

const user1 = yield User.get("123").run();
const user2 = yield User.get("123").run();

user1.must.eql(user2);

it explodes and complains about JSON serialization issues

ReqlDriverError: `toJSON` takes 0 argument, 1 provided after:
r.table("User")
      at Term._arity (node_modules/rethinkdbdash/lib/term.js:3014:11)
      at Term.toJsonString (node_modules/rethinkdbdash/lib/term.js:2765:10)
      at Query.(anonymous function) [as toJSON] (node_modules/thinky/lib/query.js:814:60)
      at Model.(anonymous function) [as toJSON] (node_modules/thinky/lib/model.js:722:32)
      at Object.stringify (native)
      at exports.stringify (node_modules/must/lib/index.js:38:19)
      at Must.assert (node_modules/must/must.js:1151:13)
      at Must.eql (node_modules/must/must.js:544:8)
      at Context.<anonymous> (test/models/user_test.js:53:19)
      at next (native)
      at onFulfilled (node_modules/co/index.js:65:19)

[moll]

Hey! Thanks for sharing. Is it me or is this actually Thinky's weird behavior when its toJSON gets called by JSON.stringify? That, or RethinkDBDash's. I suspect the problem is that JSON.stringify passes the key it's serializing the object under to toJSON, but either of the two are not prepared to handle that.

[moll]

Sections from the spec to help your argument with them:

• http://www.ecma-international.org/ecma-262/5.1/#sec-15.12.3
• http://www.ecma-international.org/ecma-262/6.0/#sec-serializejsonproperty

[kaievns]

i don't really know who's fault that is, but i can tell you that it works like a charm in both chaijs and expectjs :) Also, i can tell you that:

Object.assign({}, user1).must.eql(Object.assign({}, user2));

is a total mood killer, so I thought I'd let you know about the problem

[moll]

I wouldn't do the above either. :) It's probably not even guaranteed to return the same properties with it ignoring inherited properties and all.

I don't know why Chai and Expect work. I take it they don't call JSON.stringify. That happens only when printing the model out, right? As a quick fix I think you can overwrite Thinky's Model.prototype.toJSON to do the above Object.assign trick for you.

Anywho, I created https://github.com/neumino/thinky/issues/501 for you.

[kaievns]

thanks! :)

[moll]

To confirm just in case, the above happens only when the assertion failed and it's printing the model out for the diff, right?

[kaievns]

no, not really. it happens even when two equal records are compared (as show in the first example)

i don't think it even gets to the assertion part. it seems that the failure happens when you try to export data for comparison or something like that

[moll]

For it to get to the stringifying part of Must.prototype.assert it had to have been unequal in Must.prototype.eql's view I suspect. As you know, equality of objects in JavaScript is tricky because due to lack of traits, operator overloading or equivalent language features, we need to rely on conventions. The conventions Must's eql adheres to are based on https://github.com/moll/js-egal, with the addition that non-value objects are compared recursively if they're from the same constructor (i.e same "class"). I presume the latter is applicable to models from Thinky.

If you print the objects out with Util.inspect, do they seem equivalent to you?

console.log(require("util").inspect(user1, {depth: null}))
console.log(require("util").inspect(user2, {depth: null}))

Although Util.inspect ignores inherited properties, I doubt Thinky's mutating anything there per instance.

[kaievns]

here's what i have when i dump it

model {
  email: 'blah@blah.com',
  id: '0cb2f830-04f9-474c-b011-f4ea2b814bbe',
  passwordHash: 'JUZpKmmi5W2BmBphQnP8qThe8V4=',
  passwordSalt: 'brLUp2EOJ3yZfd7o' }
model {
  email: 'blah@blah.com',
  passwordSalt: 'brLUp2EOJ3yZfd7o',
  passwordHash: 'JUZpKmmi5W2BmBphQnP8qThe8V4=',
  id: '0cb2f830-04f9-474c-b011-f4ea2b814bbe' }

those are not ReQL query like @neumino said. those are instances of a User model. and they are identical

[neumino]

How did you declare your model?

[kaievns]

@neumino the usual

const User = thinky.createModel("User", {
  email: String,
  passwordSalt: String,
  passwordHash: String
});

is there any other way to declare it?

[neumino]

Hum, can you provide a whole script that triggers this error? I'm not super familiar with js-must, so that would help :)