Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jest plugin doesn't support inline snapshots #667

Closed
fdc-viktor-luft opened this issue Nov 25, 2021 · 2 comments
Closed

Jest plugin doesn't support inline snapshots #667

fdc-viktor-luft opened this issue Nov 25, 2021 · 2 comments

Comments

@fdc-viktor-luft
Copy link
Contributor

fdc-viktor-luft commented Nov 25, 2021

Issue

When using expect(foo).toMatchInlineSnapshot() in some test suite, jest complains in the moment it tries to inline the snapshot at the correct position of the code with:

Jest: Couldn't locate all inline snapshots.

Relations

The issue is very likely related to jestjs/jest#6744.

Further investigations

  • I have also projects were this works but I'm using the exact same versions of all packages and I can't tell the difference.
  • expect(foo).toMatchSnapshot() works without any problems (since Jest doesn't mutate the original code)
  • expect(foo).toMatchInlineSnapshot(`whatever`) works until you would tell Jest to update the snapshot

Environment

Package versions

"@sucrase/jest-plugin": "^2.2.0",
"jest": "^27.3.1",

Node version

16.13.0

@alangpierce
Copy link
Owner

alangpierce commented Nov 27, 2021

Hi @fdc-viktor-luft , thanks for flagging, I just tried it out and I agree that it's an issue. It looks like this was probably caused by #661 , where I changed the Jest plugin to include a source map as part of the output. I just tried downgrading to @sucrase/jest-plugin@2.1.1 and confirmed that that fixes the issue. (I also had to downgrade Prettier since it looks like the just-released 2.5.0 doesn't work with Jest. I also had to run jest --clearCache to bust the cached code from the previous plugin version.)

From my digging so far, it looks like here's what's happening:

For some background, Sucrase is intentionally careful to have each line of output exactly match the same line of input, so e.g. line 53 in the transformed code is line 53 in the source code and stack traces give helpful results even if you haven't set up source maps. Originally, Sucrase didn't include source maps at all because they weren't necessary, especially if you're only care about line numbers (e.g. when setting debugger breakpoints). However, some tools like WebStorm want to see some source map for transformed code, or else they don't let you set breakpoints at all (because they assume the line number would be wrong). To help support these cases, Sucrase is able to generate a simple source map where the start of each line in the output file is mapped to the start of that same line in the input file, basically an "identity" source map.

In order for Jest's inline snapshot feature to work, it needs to know where it can write the snapshot. From my digging so far, it looks like it works by catching the exception and using the stack trace information to figure out the line and column where the updated snapshot text should be placed.

In order for the stack trace information to work right, Jest automatically uses the source-map-support package, which is a node implementation of source mapping to automatically update stack trace line/column numbers. From a little debugger work, I was able to confirm that source-map-support is mapping the error to the correct line, but column 0, so Jest thinks that that's where the toMatchInlineSnapshot should be, and gives an error when it doesn't find it there. When the Sucrase Jest plugin doesn't include source maps (as in the previous plugin version), the line and column are unmodified and toMatchInlineSnapshot works.

I'm not totally sure on the best fix, but a few ideas come to mind:

  • The source map info that Sucrase generates could have column-level information. The simplest approach is to map each character position to itself, though I worry about source maps being really big in that case, and it would be incorrect when Sucrase does transform code (likely a rare issue with toMatchInlineSnapshot).
  • There may be a simple and compact way to otherwise indicate that each column maps to itself rather than all columns mapping to 0. For example, maybe additionally mapping the end of each line to itself would work.
  • Sucrase could generate truly correct column-level source maps, likely as part of TokenProcessor. The current source map system is kind of nice in that it's optional and has zero performance impact when disabled and it's nicely self-contained, but good tooling integration is certainly important and may justify the complexity.
  • It may be best to just revert the Jest behavior of including source maps or make it opt-out, though I'm hesitant to do that since it's needed for WebStorm debugging and I'd rather find a solution that makes Sucrase work well with every tool.

Downgrading the Jest plugin certainly should work as a stopgap, but I can take a look at the source-map-support details and see if there's a way to have it map columns properly for Sucrase's case.

@alangpierce
Copy link
Owner

This is now fixed with #759 (an implementation of the third bullet point in my previous comment), just released in Sucrase 3.32.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants