fix(snapshot): allow inline snapshot calls on same location with same snapshot

[jycouet]

Description

fix(inline-snapshot): can be used by multiple tests. On clean state, will take the first result.
fix: #7450

---

From 2.1.9 and after, it was not possible to have multiple tests using the same toMatchInlineSnapshot. It's the result of #6327 & #6332. I Belive that the root cause is "What to put as default value from a clean state ?" And before, the cli was adding all serialized result. This PR is fixing this by adding only the first result.

Summarized by:

test("snap", () => {
  for (const str of ["foo", "bar"]) {
    expect(str).toMatchInlineSnapshot();
    // Was doing
    // transforms to expect(str).toMatchInlineSnapshot(`"foo"``"bar"`);
    
    // After this PR:
    // transforms to expect(str).toMatchInlineSnapshot(`"foo"`);
    // and fail the second expect
  }
});

Let me know what I missed, I'm new here 👋

---

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:.

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	118ebb0
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/67be55043ff2970008c1565d
😎 Deploy Preview	https://deploy-preview-7464--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[hi-ogawa]

Hey, thanks for the PR 👋

Previously we have decided that throwing is good for consistency, but if we can figure out well-defined behavior without breaking, then it's worth revisiting. 🤔

Can you add tests in test/snapshots? The one in test/cli/fixtures/fails/inline-snapshop-inside-loop.test.ts isn't so flexible to simulate many scenarios, but you can take a look at tests like test/snapshots/test/skip-test.test.ts or test/snapshots/test/summary.test.ts to test various scenarios like initial snapshot creation, updating snapshot, etc...

[jycouet]

Thank you @hi-ogawa for the message & hints.

I looked into what you said and updated a few things (everything ^^). And added a few tests as you suggested.

Let me know what you think.

[AriPerkkio]

Previously we have decided that throwing is good for consistency, but if we can figure out well-defined behavior without breaking, then it's worth revisiting. 🤔

Totally agree. This would be great to support but it needs to be well supported. I've recently seen multiple projects utilizing inline snapshots in loops and custom test(), that will fail once they upgrade to v3. (Also I'd like to personally use this feature in many of my tests, as I want to avoid writing assertions.)

[jycouet]

@AriPerkkio you saw something to enhance on this PR to have this back ?
We are in a situation where we can't update to v3 in our case too.

[hi-ogawa]

I'll start to look into this 👍 I think there's a way to make this work, so I might push some tests and code here.

One issue came back on this PR is that the following example causes an update-rerun loop on terminal. This is something we still want to prevent and probably that's possible.
(This means that test/cli/fixtures/fails/inline-snapshop-inside-loop.test.ts test should still reject ["foo", "bar"].)

pnpm -C examples/basic test repro.test.ts -- -u

// repro.test.ts
import { expect, test } from 'vitest';

test('repro', () => {
  example(1);
  example(2);  // for example, the user only had `example(1)` initially, but added `example(2)` later.
});

// this happens same when `example(2)` is in a different test case
// test('repro2', () => {
//   example(2);
// })

function example(v: any) {
  expect(v).toMatchInlineSnapshot(`1`);
}

I've recently seen multiple projects utilizing inline snapshots in loops and custom test(), that will fail once they upgrade to v3.

This change actually already happened on Vitest 2, so it wasn't even considered a breaking change 🙃

[jycouet]

Yeah, it's not a breaking from 2.1.9 to 3. It's a regression, breaking between 2.0.5. and 2.1.9 (somewhere 🫣)
But we will manage to fix it 💪

I let you continu on this ? Or you need something from me ?

[pkg-pr-new]

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@7464

@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@7464

@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@7464

@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@7464

@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@7464

@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@7464

@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@7464

@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@7464

@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@7464

@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@7464

@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@7464

vite-node

npm i https://pkg.pr.new/vite-node@7464

vitest

npm i https://pkg.pr.new/vitest@7464

@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@7464

@vitest/ws-client

npm i https://pkg.pr.new/@vitest/ws-client@7464

commit: 9e4dfd2

[hi-ogawa]

@jycouet So far so good. As you can see in the diff, this simply relaxes the error condition, so it's a matter of whether this is relaxing enough for the desired use cases to work again. Please feel free to try and let me know if you spot something 🙏

[jycouet]

Our CI is running well with what you deployed with pkg-pr-new 🙏
Great tests addition, I didn't spot any thing special.

I'm wondering why some tests are failing (Looks like not linked with what we are doing). It's some flaky tests ? 🤔
CI / Browsers: node-20 ?

[hi-ogawa]

Browser mode especially on Windows has been flaky, so probably nothing to worry.

[jycouet]

So, it all looks good to me 👍
Thanks for guiding me & adding all these tests.

[jycouet]

@hi-ogawa could yuo tell me what are the next steps ? You need anything else from me ?

[hi-ogawa]

This looks good to me. I'll wait for another review from team just in case.

[sheremet-va]

Looks good to me!