[Bug]: New SERIALIZABLE_PROPERTIES is only used by matcher-utils

[stephenh]

Version

30.0.0-alpha.4

Steps to reproduce

Setup a test with the new SERIALIZABLE_PROPERTIES feature:

  it("tests foo", () => {
    class Volume {
      constructor(amount, unit) {
        this.amount = amount;
        this.unit = unit;
      }

      get label() {
        throw new Error("Not implemented");
      }
    }
    Volume.prototype[SERIALIZABLE_PROPERTIES] = ["amount", "unit"];
    expect(new Volume(10, "L")).toEqual(new Volume(10, "L"));
  });

And set a breakpoint in the expect-utils eq function.

Expected behavior

I expected toEquals and eq to use the SERIALIZABLE_PROPERTIES for the equality checked, but it is only checked for the diff formatting.

Actual behavior

SERIALIZABLE_PROPERTIES is actually only checked in the printDiffOrStringify method.

Additional context

I work with an ORM that creates deeply-interlinked entities, and so have been wanting to customize the toEqual functionality for awhile now--particularly our diffs toEqual failures can get huge.

The new SERIALIZABLE_PROPERTIES looked really neat, but I was surprised to see it's only used by the diff printer, and not the core eq logic itself.

I admittedly do not have a bug reproducing exactly why that's problematic, but it does just seem unintuitive? Either from two perspectives:

• The name SERIALIZABLE_PROPERTIES to me infers it will be used "for serialization" which I would have expected to apply to both "the toEqual check" as well as "the toEqual diff", but also
• If the core expect-utils eq logic did not need SERIALIZABLE_PROPERTIES to avoid blowing up on the getter from the original PR's use case (which FWIW we've definitely hit that bug too, that's why I care about this), then why did matcher-utils need a new custom property? Couldn't matcher-utils have just used expect-utils logic?

I guess I don't have a true bug per se, so happy to close this out @georgekaran / @SimenB , but it just seems a little disjoint, i.e.:

• custom equality checkers are hook into toEqual eq but not the diff creation
• SERIALIZABLE_PROPERTIES hooks into the diff creation, but not toEqual eq

I totally get evolving ubiquitously-used software over-time is basically impossible 😓 , but just seems like there is a lot of disjointness, and maybe SERIALIZE_PROPERTIES as a user-observable flag wasn't a necessary addition, if matcher-utils-the-diff and expect-utils-the-equality reused more of their core functionality.

Again, not a bug per se, just an observation that SERIALIZABLE_PROPERTIES doesn't work the way I expected, and I think for our ORM usage we'll either have to do both custom equality checker + SERIALIZEABLE_PROPERTIES to get toEqual to do what we want. Which is fine too.

Thanks!

Environment

Jest v30.0.0-alpha.4

[SimenB]

I think it makes sense to align, but I'll defer to @georgekaran

[georgekaran]

Hey @stephenh

Your points make a lot of sense when it comes to the SERIALIZABLE_PROPERTIES. To be honest, maybe the name SERIALIZABLE_PROPERTIES wasn't the best choice 😅, which might have led to some confusion as it seems to imply a broader scope than it currently has.

That being said, I think we could go to either two directions:

• Rename this property to make it clear that is only used by the diff printer;
• Apply this same variable to equality checkers.

Personally the second option makes more sense to me, but is probably gonna take a lib bit more time than just a renaming.

Let me know what you guys think, so I can start working on a draft to address this issue and aim to include it in a future pull request.