jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.2k stars 6.46k forks source link

[Bug]: Strings unexpectedly have no escaping in snapshots? #13776

Open lydell opened 1 year ago

lydell commented 1 year ago

Version

29.3.1

Steps to reproduce

  1. Clone this gist: https://gist.github.com/lydell/054071e15b38d9223f6bb026d6c6a73c
  2. Look at the inline snapshots in index.test.js.
  3. Optional: Run npm it to verify the snapshots.

Expected behavior

I expected double quotes in strings to be escaped in snapshots.

Actual behavior

Double quotes are not escaped.

Additional context

I updated from Jest 28 to 29 and was surprised seeing this in my snapshot after updating it:

      {
        "closed": true,
        "type": "StringLiteral",
        "value": """",
      },

https://github.com/lydell/js-tokens/commit/368dd65d3e126b2d8d9283ef4c141c0f027aa9c2#diff-fe86fc96fcb8e216d5f567cd722b97c78783f153bb86e01b82e02fad7d1354fdR32

I was like – “""""? What’s up with that?” And then “Ooooh! It’s the empty string … as a string … in a snapshot string 🙃 ”

Wanted to check if this is expected behavior or not?

It allows you to do fun things like this – how many properties does the object have?

test("Bobby", () => {
  const bob = {
    name: `Bob Drop",\n  "nick": "Bobby Tables`,
    occupation: "Student",
  };
  expect(bob).toMatchInlineSnapshot(`
{
  "name": "Bob Drop",
  "nick": "Bobby Tables",
  "occupation": "Student",
}
`);
});

Environment

System:
    OS: macOS 13.1
    CPU: (10) arm64 Apple M1 Pro
  Binaries:
    Node: 18.9.0 - ~/.local/share/nvm/v18.9.0/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.19.1 - ~/.local/share/nvm/v18.9.0/bin/npm
  npmPackages:
    jest: 29.3.1 => 29.3.1
SimenB commented 1 year ago

Yeah, this is the major breaking change of Jest 29: https://jestjs.io/blog/2022/08/25/jest-29

SimenB commented 1 year ago

Ergh, your example doesn't look too good, tho 😅 Anything we can do about that? I don't think we want to escape newlines in a string literal - I have a bunch of e.g. table or csv inline snapshots at work that should definitely render out the newlines and not escape them.

/cc @orta

orta commented 1 year ago

I think we probably want to look for newlines in the value and switch the outer character from a quote to an escaped backtick? That can keep the multi-line-ness and still describe the text - it also means the code can still be C&P'd in theory, though the backticks would get lost

But at least looking at the test, the string it is representing is the string "" so """" is a silly but reasonable answer because you wouldn't want it to be "" 🍡

lydell commented 1 year ago

Btw, I really like the snapshot defaults in Jest 29, makes snapshots much easier to read in general.

If we can come up with some clever way to improve these edge cases that might be nice. Otherwise it might just be about getting used to it.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.

lydell commented 1 year ago

Hi @github-actions! See you again in a month or two!

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 30 days.