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

Set contentWindow default ourselves to avoid Jest serialization #260

Merged

Conversation

cameronmcefee
Copy link
Contributor

This fixes #249

Jest serializes all props in snapshots. The current code sets window by default, which is a huge object. When Jest tries to serialize it, it typically throws RangeError: Invalid string length because the process runs out of memory. If you're lucky enough to actually get a test to pass, the snapshot will be around 250mb.

This switches from using defaultProps to setting the default ourselves, so in the test environment there won't be a prop value to serialize.

I confirmed the commonjs version fixes the issue. I'm not sure how to test the es6/umd versions. npm test passes.

Jest serializes all props in snapshots. Window is a huge object, so when Jest tries to serialize it, it typically throws `RangeError: Invalid string length` because the process runs out of memory. This switches from using defaultProps to setting the default ourselves, so in the test environment there won't be a prop value to serialize.
Copy link
Owner

@clauderic clauderic left a comment

Choose a reason for hiding this comment

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

Hey @cameronmcefee,

Thanks for taking the time to submit this PR ❤️

LGTM 🚢

@clauderic clauderic merged commit e599da7 into clauderic:master Aug 8, 2017
@cameronmcefee cameronmcefee deleted the set-window-default-manually branch August 8, 2017 03:53
@clauderic
Copy link
Owner

Just published this fix with 0.6.7

@cameronmcefee
Copy link
Contributor Author

❤️ Thanks, this component has saved me much time. I'm happy to have contributed.

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.

Out of memory when snapshotting react-sortable-hoc
2 participants