fix(expect): Fix to properly compare ObjectContaining

[pengooseDev]

Description

Fixes #5714

This PR ensures the ObjectContaining matcher compares objects properly. It fixes an issue where extra properties in the actual object were not correctly handled, ensuring only the specified properties in the sample are compared.

What this PR solves

• Proper comparison of specified properties in ObjectContaining.
• Accurate snapshot output reflecting expected vs actual differences.

Additional Context

• Updated matcher logic to handle missing keys.
• Adjusted test snapshots for correct comparison logic.

Prev

Chaged

---

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

[sheremet-va]

Not sure if you've seen this, but did you have this in mind for this PR?

vitest/packages/utils/src/error.ts

Line 111 in 7cbd943

const { replacedActual, replacedExpected } = replaceAsymmetricMatcher(clonedActual, clonedExpected)

[pengooseDev]

@sheremet-va
I'll check that soon. Thx :)

[pengooseDev]

@sheremet-va
I apologize for the delayed response. I am currently trying to manage the logic within the replaceAsymmetricMatcher as per your feedback. However, I was unable to dedicate as much time as expected due to other commitments over the weekend.

If you wish to temporarily apply this PR, it would be good to merge it first and then recreate the issue to discuss the direction of moving the logic into replaceAsymmetricMatcher.

Additionally, I understand that applying the logic within replaceAsymmetricMatcher affects all expect methods. I would like to confirm if this is an approach to minimize external dependencies (jest).

[pengooseDev]

@sheremet-va

I would greatly appreciate any feedback and guidance you might have on the current changes. Additionally, if you feel that these changes do not align with your vision or the work cycle is too lengthy, please feel free to close this PR. :)

Context

I have considered the feedback you provided previously. My understanding is that you aim to minimize external dependencies (such as jest) and internalize the technology. You also appear to be interested in extending the application to all asymmetricMatchers, not just ObjectContaining.

However, I also believe it is important that this PR does not introduce too substantial changes. Therefore, the following modifications have been applied:

---

1. Change of Abstraction Location

• Moved the logic to address the issue from inside ObjectContaining to 'replaceAsymmetricMatcher' in utils/error.ts.

---

2. Adding 'err' Parameter to replaceAsymmetricMatcher

This additional parameter is intended only for changes in ObjectContaining among asymmetricMatchers. The variable is used as follows:

// utils/error.ts
function isObjectContaining(err: any): boolean {
  return err
    && typeof err.expected === 'object'
    && typeof err.expected.asymmetricMatch === 'function'
    && (err.expected.toString() === 'ObjectContaining' || err.expected.toString() === 'ObjectNotContaining')
}

export function replaceAsymmetricMatcher(
  actual: any,
  expected: any,
  actualReplaced = new WeakSet(),
  expectedReplaced = new WeakSet(),
  err?: any,
) {
  // ...codes
  if (isObjectContaining(err)) {
    getOwnProperties(actual). forEach((key) => {
      if (!Object.prototype.hasOwnProperty.call(expected.sample, key)) {
        expected.sample[key] = actual[key]
      }
    })
  }

• The 'err' parameter can be removed if the new diff logic is applied to all asymmetricMatchers.

---

3. Additional Questions

I would like to ask if it is acceptable to use an object as a function parameter in the manner below, to reduce human error and eliminate code like 'actualReplaced: undefined'.

    // example
    const { replacedActual, replacedExpected } = replaceAsymmetricMatcher({
      actual: clonedActual,
      expected: clonedExpected,
      err
    })

[sheremet-va]

What I was asking is if you think the code in replaceAsymmetricMatcher is related to your changes (I don't know if it is). I am not asking you to move the code around. If you think they are unrelated (you change the code behavior, not the display), we can proceed with the PR.

[pengooseDev]

@sheremet-va
The previous changes had a side effect of modifying the expected values and messages in snapshots. This was due to the changes made to the original expected values.

Origin

• test/core/test/snapshots/jest-expect.test.ts.snap

  "expected": "ObjectContaining {
  "k": "v",
  "k3": "v3",
}",
  "message": "expected { k: 'v', k2: 'v2' } to deeply equal ObjectContaining {"k": "v", "k3": "v3"}",
}
`;

Changed

  "expected": "ObjectContaining {
  "k": "v",
  "k2": "v2", # ⛳️ added
  "k3": "v3",
}",
  "message": "expected { k: 'v', k2: 'v2' } to deeply equal ObjectContaining{…}", # ⛳️ changed

The current change remove this side effects. Furthermore, using the flag (err.expected.toString()) appropriately seems to allow scalable responses to the same issues with ArrayContaining, and it would be advisable to separate this logic into a separate function.

expect([1, 2, 3]).toEqual(expect.arrayContaining([2, 4]))

AssertionError: expected [ 1, 2, 3 ] to deeply equal ArrayContaining [2, 4]

- ArrayContaining [
+ Array [
+   1, // ⛳️ should be changed
    2,
-   4,
+   3,
  ]

---

Remaining Issues with the Current Code

The method of adding non-comparison values to expected is not being handled recursively. The final comparison in the diff function is done using Object.is, which still poses problems for depth 2 reference data (Objects, WeakMaps, Arrays, etc.).

---

There are five possible approaches:

1. Supporting only for depth 1 objects in the current state.
2. Support functions that parse reference data of depth 2 or higher that includes mixed objects and arrays (this might not be easy when multiple reference data types are involved).
3. Discard the current changes and attempt to handle them internally in the display.
4. Put this issue on hold. (close the PR)
5. Adopt the previous approach despite its side effects. (Not recommended)

I would follow any direction you propose. I do not wish to harm this excellent open source project with my imperfect contributions. thx for your time. :)

[pengooseDev]

@sheremet-va
you change the code behavior, not the display < It was meaningful feedback that provided me with better direction. thx

[sheremet-va]

Looks like tests are failing

[sheremet-va]

What are the performance implications of this? It's the second time we deeply cloned the object, and in replaceDiffData we only iterate the first level

[pengooseDev]

This was intended to reduce unexpected side effects, but since those values are not included in the return value, it would be better to remove them as you suggested.