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

fix: no-large-snapshots to allow maxSize of 0 #132

Conversation

paularmstrong
Copy link
Contributor

Problem: Configuring the rule jest/no-large-snapshots with [{ maxSize: 0 }] sets the line limit to 50 instead of 0.

Solution: Treat 0 as a requirement for no snapshot.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sending a PR! I left a small inline comment, other than that LGTM

(context.options[0] && context.options[0].maxSize) || 50;
context.options[0] &&
typeof context.options[0].maxSize === 'number' &&
isFinite(context.options[0].maxSize)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing!

@@ -22,7 +26,9 @@ module.exports = {
if (lineCount > lineLimit) {
context.report({
message:
'Expected Jest snapshot to be smaller than {{ lineLimit }} lines but was {{ lineCount }} lines long',
lineLimit === 0
? 'Expected to not encounter a Jest snapshot but was found with {{ lineCount }} lines long'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one!

Signed-off-by: Paul Armstrong <parmstrong@twitter.com>
@paularmstrong paularmstrong force-pushed the parmstrong/allow-zero-for-snapshot-maxsize branch from ef10b64 to 7e5e4e3 Compare August 9, 2018 22:12
@SimenB SimenB merged commit e42c9e3 into jest-community:master Aug 9, 2018
@SimenB
Copy link
Member

SimenB commented Aug 9, 2018

🎉 This PR is included in version 21.20.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@paularmstrong
Copy link
Contributor Author

Thanks @SimenB !

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

Successfully merging this pull request may close these issues.

None yet

2 participants