[Breaking change] Properly escape snapshots

[vjeux]

There were many issues with snapshot test escaping:

• When escaping backtick, it didn't escape backslash
• Double quote escaping was overzealous and also escaped single quotes
• JSX text didn't use HTML escaping
• Regex escaping didn't escape all the control characters that should be

Because of the first one, you couldn't do eval(snapshot) and get the correct value, in order to make it kind of work, an unescape function was created that did ... something. Now that escaping is done properly we don't need it anymore.

This is a breaking change as all the double quotes now need to be escaped which triggers a ton of failures.

Fixes #1789

[jlongster]

Since this is going to affect existing snapshots, would it be easier to try and make it so it doesn't escape the single/double quotes? It doesn't need to because they are inside a string template. Shouldn't it only have to escape the backtick?

I think all existing snapshots will be less affected, if the above is possible.

[vjeux]

I actually tried making the quotes be backticks but then it looks really weird.

<div className=`my-class` />

or

{key: `some string`}

But, now that you mention it, I'm not really sure why everything is wrapped in a backtick string. If the value is a string, we could just write that string instead of writing a string that contains a string and pay the cost of escaping twice.

[jlongster]

But, now that you mention it, I'm not really sure why everything is wrapped in a backtick string. If the value is a string, we could just write that string instead of writing a string that contains a string and pay the cost of escaping twice.

Yeah, that could work. But if we keep it wrapped in a backtick, why do we need to escape at all? This is valid:

` "

' " " `

Single or double quotes within a template don't need any escaping... might be missing something though.

[vjeux]

Right now, .toMatchSnapshot('this " is a quote ') generates

exports['key'] = `
"this \" is a quote"
`;

What I'm suggesting is to instead output

exports['key'] = 
`this " is a quote`;

[jlongster]

Oh, yeah that looks great and should have minimal impact on existing snapshots.

[cpojer]

How is that going to work for snapshots dealing with objects or arrays that have strings in them?

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.