Improve comparison/diff

[mindplay-dk]

As reported in #120, I ran into a problem with Assert.equal, where the assertion decided the objects were equal, but the diffReporter was unable to report any difference.

I finally isolated the problem: one of (what I thought was) my JSON models had a Date in it - this is fine when passed through JSON.stringify which produces exactly the date format I was expecting, so this was rather tricky to isolate.

I now have a repro:

  is.equal(
    { ouch: new Date("2222-01-01T00:00:00.000Z") },
    { ouch: "2222-01-01T00:00:00.000Z" }
  );

So the assertion fails, but diffReporter disagrees and reports no difference:

This applies to object comparisons only - so this behavior isn't consistent with value comparisons, for example:

  is.equal(
    new Date("2222-01-01T00:00:00.000Z"),
    "2222-01-01T00:00:00.000Z"
  );

This works as expected:

The test in fast-deep-equal where this fails is a && b && typeof a == 'object' && typeof b == 'object', because typeof b is string and not object - so the library is right, a Date is not equal to a string, completely reasonable.

So the problem is diffReporter, which doesn't recognize this difference.

I suspect the problem is here:

zora/reporters/src/diff/diagnostic/equal.js

Lines 148 to 160 in a68f308

	return (diag) => {
	const { actual, expected } = diag;
	const expectedType = typeof expected;
	if (typeof actual !== expectedType) {
	return differentTypes({ actual, expected });
	}
	return (
	sameTypeDiff[expectedType]?.(diag) ??
	`unsupported type ${theme.emphasis(expectedType)}`
	);
	};

Here it decides that the objects are equal types because they're both object - it then takes sameTypeDiff[expectedType] where expectedType is string, and so they get diffed as strings.

diffStrings passes this along to diffChars from the diff package:

zora/reporters/src/diff/diagnostic/equal.js

Lines 32 to 46 in a68f308

	const diffStrings = (theme) => {
	const diffChars = getDiffCharThemedMessage(theme);
	return ({ expected, actual }) => {
	const { expected: expectedMessage, actual: actualMessage } = diffChars({
	expected,
	actual,
	});
	return `diff in strings:
	${theme.errorBadge('- actual')} ${theme.successBadge('+ expected')}
	${theme.errorBadge('-')} ${actualMessage}
	${theme.successBadge('+')} ${expectedMessage}`;
	};
	};

My guess is, it was asked to diff a string, so it converts the Date to a string, and there you go.

I'm unsure precisely how we can fix this, but I would think it starts with the type comparison in diff/diagnostic/equal.js, which, yes, compare the types, but something else is missing here.

I call having a similar problem in the past, where I ended up doing a custom serialization to strings - something JSON-like but with types, so:

class User {
  constructor(name) {
    this.name = name;
  }
}

console.log(serialize({ user: new User("Bob"), date: new Date() });

Would output something like:

{
  user: User {
    name: "Bob"
  },
  date: Date("2222-01-01T00:00:00.000Z")
}

Special formatting is still required for Date since it's a sort of "value-typed object".

For the same reasons, for Node, there is util.inspect, which produces an output similar to this:

import { inspect } from "util";
console.log(inspect(new User("Bob"))); // => "User { name: 'Bob' }"

Of course, we would need support in the browser as well, so we can't depend on Node's util.inspect - a quick search reveals this replacement package, which explains:

It was built in such a way that it can be kept up-to-date with node's implementation, by taking the code directly from node's repo, and changing nothing but the require() statements. All of the node built-in functions are emulated. Many of the incompatibilities generated from that emulation are not interesting for Web use cases.

One more use-case to demonstrate the seriousness of this problem:

  class User {
    name: string;
    constructor(name: string) {
      this.name = name;
    }
  }

  class Login {
    name: string;
    constructor(name: string) {
      this.name = name;
    }
  }

  is.equal(new User("Bob"), new Login("Bob"));

  is.equal({ user: new User("Bob") }, { user: new Login("Bob") });

Both of these tests will fail, reporting no difference.

[lorenzofox3]

It is a bit similar to the object equality question although here it relates on how things get serialized.

The diff algorithm (a great third party library) works on the JSON specification.

Dates, Prototypes, etc have nothing to do with the JSON specification so it serializes things first loosing information (how would you encode a constructor or a prototype in a JSON for example ? ).

If you want to get full control you'd need to take the control on the serialization to enrich the information with JS meta : constructors, prototypes, etc; then find a format to print it properly and compare the result as strings only. That's a lot of work and you can't simply cover everything: for example a lot of reporting tools use the constructor but quid of objects created with Object.create ? what about objects with a long prototypal chain ? should you go up on non enumerable properties ? Many questions you can't answer easily and I am not sure it worths the effort (look at the various reporting packages: these are megabytes of code).

Also It can't simply be consistent: your "serious" problem would not even fail in Jest. Ava, would report a constructor mismatch but if you write the code

const LoginProto = {};
const UserProto ={};

const createLogin = ({name}) => Object.create(LoginProto, {name:{value:name}});
const createUser = ({name}) => Object.create(UserProto, {name:{value:name}});

and compare a "Login" with a "User" the test would not fail either whereas semantically it is the same code than the one written with classes

JSdiff handles the complexity of the serialization and the diffing, it has its tradeoffs of course, but I am quite ok with it. Maybe we can work on something specific for native JS classes such Dates, Buffers, etc. But anyway if the test fails whereas you don't see any diff, it simply means you have a type mismatch and TS should catch it, should not it ?

[lorenzofox3]

you can have a test playground in the following gist