fix: Support diffing single-line strings

[thymikee]

Fixes #22
Also simplifies our test components btw, because it annoyed me it's this long unnecessarily 😛.

[pedrottimark]

If one value is single-line and other value is multi-line, does the if statement append newline to both?

How about separate if statement for each value which tests RegExp only if typeof string? Although this seems like the primary use case, from quickly reading code in index.js of jest-diff package (but not trying it, I admit :) I think the snapshot-diff assertion works for other objects, not just React elements, true?

if (typeof valueA === 'string' && !MULTILINE_REGEXP.test(valueA)) {
  valueA += '\n'; // eslint-disable-line no-param-reassign
}
if (typeof valueB === 'string' && !MULTILINE_REGEXP.test(valueB)) {
  valueB += '\n'; // eslint-disable-line no-param-reassign
}

[thymikee]

Yea, makes sense, addressed

feedback addressed

[SimenB]

this seems weird, IMO. Maybe not, though :D

[thymikee]

Maybe that's a regression?
jestjs/jest#1633
cc @pedrottimark

[thymikee]

Hah, there's even a test for that.

Is this because jest-diff was supposed to provide an inline diff, but it was not supported at the time (and nothing change in that matter for now)?

[pedrottimark]

Y’all ask a good question about this special case. I added test to document it, not endorse it :)

I guess it’s because jest-diff omits output that seems redundant when expect calls it:

but it’s confusing when snapshot-diff calls it.

Not to sound critical of awesome effort to integrate expect matchers with diff display, and then snapshots, it feels like the whole is less than the sum of the two parts in some cases. What do you think if expect omits Expected value to equal and Received lines when a diff follows? EDIT: Then jest-diff could drop the special case, as a breaking change.

Yesterday I updated a local copy of code from dormant Jest PR for inline substring diff to call diff-sequences package. This version is somewhat easier to grok than the original :)

Any feedback how to expose that new function for Jest and other callers is very welcome.

[thymikee]

@pedrottimark you could expose some useful utils like diff lib (talking about something like diffLines and structuredPatch we use).

Back to the topic, diff + expect behavior feels like a hack to me. It's .toEquals() matcher that should be responsible for determining whether to display the diff or not.
Do you think it makes sense to file a PR to Jest?

[pedrottimark]

Yeah, we might refactor to more clearly separate assertion logic at one extreme and diff display at another extreme from the responsibility you mention, which I think is divided between expect/src/matchers.js and jest-diff/src/index.js files.

[pedrottimark]

Have started on first of a series of pull requests for expect and jest-matcher-utils which are prerequisite to end goal to remove limitation from jest-diff so it returns a diff for single-line strings.

[thymikee]

Oh, great, thanks for the heads-up! boolean and number need these updates too.

[thymikee]

Closing, this is fixed upstream in Jest 23. Thank you all for chiming in!