comments in snapshots

[benmonro]

🚀 Feature Proposal

My team has a lot of large snapshot files. I would like to add the jest/no-large-snapshots rule but I need to grandfather in the existing snapshots as there are too many of them to convert without sinking a ton of time into it. I can put //eslint-disable-next-line in the snap file but when you run -u or add a new snapshot, all of those comments are removed from the snap file. So would be nice to be able to add an option either in .toMatchSnapshot() or in the snap file itself which would ensure that these comments do not get removed unless the snapshot actually changes.

Motivation

to be able to use //eslint-disable-next-line comments in snapshot files.

Example

.toMatchSnapshot({comment: "//eslint-disable-next-line") maybe?

or maybe just

//eslint-disable-next-line jest/no-large-snapshots
exports[`My big snapshot that won't go away, even though I'm stomping my feet`] = `...`;

Pitch

Why does this feature belong in the Jest core platform?
Because snapshots are in jest?

Common feature proposals that do not typically make it to core:

• New matchers (see jest-extended)
• Changes to the default reporter (use custom reporters instead)
• Changes to node/jsdom test environments (use custom environments instead)

[thymikee]

I'd say a big no to this. The proper way to do it is to change the way the no-large-snapshots eslint rule works to link to the .toMatchSnapshot() call, same as it does for .toMatchInlineSnapshot(). Please transfer this issue to https://github.com/jest-community/eslint-plugin-jest :)

[benmonro]

@thymikee I'm not sure I understand the quick trigger finger on closing this issue. There's no way for a lint rule to ignore specific instances of a violation. It can ignore an entire file, but not specific lines for example. Is there a way to accomplish this in jest with a custom serializer perhaps? Or is there a way to tell jest to not wipe out a comment if I add it to the snap file?

[thymikee]

Why not? If eslint plugin has a knowledge of file path, then it can infer the snapshot location inside the default __snapshots__ directory adjacent to the file or using snapshotResolver config if present.

[benmonro]

@thymikee because a snapshot file can have multiple snapshots, and I can't disable the rule for the entire file in my use case. The only way to disable a lint rule for a particular line is by adding a comment on the line above it //eslint-disable-next-line. but since snapshot files are generated, it wipes out those comments when you run -u or add a new snapshot, even if the snapshot w/ the comment didn't change.

[thymikee]

I still don't see this as a limitation for eslint plugin. I'm also against any special treatment for eslint stuff in snapshots.

The way I see it:

// file.js
test('a', () => {
  // eslint-disable-next-line
  expect(bigData).toMatchSnapshot();
}

test('b', () => {
  expect(bigData).toMatchSnapshot('c');
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: snapshot too big, [link to snapshot]
}

eslint plugin logic:

1. find toMatchSnapshot
2. find corresponding test name
3. construct full test name
4. resolve snapshot (using snapshotResolver)
5. display errors where appropriate

// __snapshots__/file.js.snap
exports[`a`] = `
...
`

exports[`b: c`] = `
^^^^^^^^^^^^^^^^^^^ - optionally error here if links won't work
...
`

It's all there, no need to change anything in the core.

[benmonro]

it's not really special treatment to eslint, i just want a way to have comments before a snapshot so they won't get wiped out. i'll try to fork eslint-jest and see if i can make it work the way you mentioned.

[benmonro]

@thymikee can you point me to the docs on snapshotResolver?

[thymikee]

https://jestjs.io/docs/en/configuration#snapshotresolver-string

cc @SimenB

[benmonro]

@thymikee so in the context of eslint-plugin-jest, a project can configure lots of things (snapshot folders, test folders/extensions etc), so wondering if you know of a way that I can just get the jest configuration for a given project and then use that in the resolver as you mentioned.

In otherwords, in order to be able to add a 'toMatchSnapshot' handler which is then able to go into the .js.snap file I need a generic way to get the jest config in order to have a reliable resolver. does that make sense? is there any way to do this w/ jest or some other package? i.e. getJestConfigForProject('./path/to/app')

[thymikee]

You should be good with jest-config and readConfigs method

[benmonro]

@thymikee so (after some delay) I was able to get jest-config & readConfigs working, however, I'm facing another challenge that I was hoping you might have some advice on.

So resolving & associating a test w/ a snapshot is pretty straight forward when you have a normal test w/ a string for the title: test('should do stuff', () => {})

but things get hairy when a test uses a template literal, for example in conjunction with describe.each:

describe.each(someArrayOfThings)("desc", (item) => {
   test(`look for ${item} in store`), () => {
      expect(item).toMatchSnapshot();
   }); 
});

so was hoping perhaps you might have some thoughts on how you could associate a snapshot to a test or visa versa. like what if the items in someArrayOfThings aren't unique?

was hoping perhaps you might have some thoughts/ideas? :)

[thymikee]

Huh, that makes it harder! You'd probably need to pre-evaluate the whole file, find evaluated strings, gather source maps and then point back to eslint. Complex and not really optimal though, especially that the job is done on nearly every key stroke.

I'm gonna reopen this and let's think of a way on how to introduce it. I'd rather like .toMatchSnapshotWithComment() because .toMatchSnapshot signature is already pretty complex and depends on what's passed (see "property matchers").

[thymikee]

But I still think the basic scenario would be beneficial for ESLint plugin, so I'd really love to see it contributed there :)

[SimenB]

1. We should have a simple api which is just readConfig(cwd: string = process.cwd): Promise<JestConfig>
2. Not sure how to best get all test names, you essentially need to run Jest against the file to collect tests, but not execute them. Not sure what API, if any, makes sense for that (as it depends on the runner, which for jest is either jasmine or circus)

[benmonro]

So I was able to come up with this solution which adds a config option to white list snapshots per file and per name. jest-community/eslint-plugin-jest#288

Not sure if that changes your guys opinions on the matter but I'm open to either

[github-actions]

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

[github-actions]

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

[github-actions]

This issue 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.