Use inline snapshots

[billyjanitsch]

Closes #116.

WIP as it doesn't yet cover everything that the current test suite does.

Some notes:

• I like the result, especially after abstracting the transform helper.
• Not sure if formatting the output with Prettier is a good call (trade-off on conciseness vs. capturing the exact Babel output rather than just the AST).
• In some places, it might be nice to asset expect(output).toBe(input) rather than taking a snapshot when we explicitly want Babel not to touch something. To do this, we'd have to format the input string with Prettier as well, so this would need to live in transform rather than the serializer.
• The reversion to Jest 26 is due to Inline snapshots are indented at col 0 jestjs/jest#11459. I'll switch it back when that's fixed, hopefully before merging this PR.

Putting this up for a review of the approach before I replicate the rest of the existing tests.

[nikita-kensho]

Instead of defining variables for transforms, let's define a single variable for the snapshot being compared against. This would help structure the expect assertions below in the same way as they are being used in some of the other test files, by keeping the transform params close to the assertion.

[nikita-kensho]

Alternatively, we could defined a meta-assertion that can accept a list of transforms and compare them against a single snapshot. This way we could avoid thinking about variables entirely

[billyjanitsch]

Instead of defining variables for transforms, let's define a single variable for the snapshot being compared against.

Could you show a snippet of what you mean?

[nikita-kensho]

I was thinking along the lines of

[
  transform({code, options, env: 'development'}),
  transform({code, options, env: 'production'}),
  transform({code, options, env: 'esm'})
].forEach(snapshot => {
  expect(snapshot).toBe(`
    function _objectWithoutPropertiesLoose(source, excluded) {
      if (source == null) return {}
      var target = {}
      var sourceKeys = Object.keys(source)
      var key, i
      for (i = 0; i < sourceKeys.length; i++) {
        key = sourceKeys[i]
        if (excluded.indexOf(key) >= 0) continue
        target[key] = source[key]
      }
      return target
    }
    var _obj = obj,
      foo = _obj.foo,
      rest = _objectWithoutPropertiesLoose(_obj, ['foo'])
  `)
})

when we assert that multiple transforms map to a single snapshot. This avoids having to trace which snapshot variable is the source of truth, and their relations

[billyjanitsch]

I think maybe it wasn't clear in the PR body that the idea of inline snapshots is that you don't write the snapshot, Jest does. Unlike regular snapshots, it writes them inline in the assertion call instead of into a separate file.

I don't think it makes sense to consider solutions that can't auto-generate and update the output. (Each inline snapshot assertion can only be called once, by design.)

[nikita-kensho]

Gotcha, wasn't aware of the limitations of inline snapshots. Is it still possible to compare the transform outputs against some source of truth without storing them as variables? Eg expect(transform(...)).toBe(development)

[billyjanitsch]

I think I'm a bit confused by the line of questioning in this thread. Is development something like const development = transform(...)? If so, then you can definitely make that assertion because transform is a regular function that returns a string, so you're comparing two strings together. But then you've stored the transform output as a variable, which it seems like you're trying to avoid, although I don't necessarily understand why. Did you mean to write .toMatchInlineSnapshot? If so, then I don't understand what development would be.

Maybe it would be helpful to back up; could you explain broadly what you dislike about the current code, without getting into potential solutions?

That said, it seems like the consensus is otherwise to go with the #118 approach so maybe this is moot.

[joeybrk372]

Not sure if formatting the output with Prettier is a good call (trade-off on conciseness vs. capturing the exact Babel output rather than just the AST)

I like this. I think formatting with Prettier makes a developer more likely to actually read and comprehend a failed snapshot rather than just regenerating it.

In some places, it might be nice to asset expect(output).toBe(input) rather than taking a snapshot when we explicitly want Babel not to touch something. To do this, we'd have to format the input string with Prettier as well, so this would need to live in transform rather than the serializer.

I think I'm missing something. For this example wouldn't it be fine as is since expect().ToBe() is not using the snapshot serializer?

[billyjanitsch]

For this example wouldn't it be fine as is since expect().ToBe() is not using the snapshot serializer?

Yeah sorry, I didn't explain that clearly. Consider something like this:

const code = `
  foo({a, b})
`
expect(transform({code})).toBe(code)

There are two issues that will cause this assertion to fail even if the AST is identical:

1. The input code has extra indentation and newlines ('\n foo({a, b})\n'). Could be fixed using something like endent.
2. Babel will often modify whitespace even without changing the AST, e.g., the above might transpile to 'foo({\n a, b\n}'. This is harder to fix consistently.

I see two solutions. We could parse the input/output ASTs and compare those rather than code strings. Or, we could run the input string through Babel without the preset, then through Prettier, to get an input string that is more likely to have the same whitespace as the output. (The latter is what I meant in the original note.) Not sure whether this type of comparison is valuable enough to warrant the complexity of either option.

[joeybrk372]

Gotcha, I see what you mean now. I've never needed to do that type of assertion before so agreed that probably not worth adding the complexity until we run into a case that does require it. I would lean towards the latter solution though if/when we do.

[nabeel-]

I'm partial to the format of #118 since it feels like less boilerplate to sift through to see the end result. In general though I prefer the inline snapshots since it's easier to see the context of the test coupled with the result.

[billyjanitsch]

Seems like the preference is for #118 so I'm going to merge that PR into this branch, and then finish migrating the existing tests when I find the time. Thanks for the thoughts, everyone!