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

Incorporate destroy-circular logic. Fixes #2. #3

Merged
merged 1 commit into from
Nov 24, 2015

Conversation

jamestalmage
Copy link
Contributor

For serialized AssertionErrors, it is possible that the actual
and expected properties contain circular references, causing
a thrown Error when you serialize the result with JSON.stringify().

This destroys circular references on the copied Object. Replacing
them with a string ([Circular]).

Also, JSON.stringify(function() {}) returns undefined, and
will drop any functions that are attached to an object it is serializing.
This mimics the latter behavior, but if a function is passed in as the top
level argument, it returns a string ([Function: fnName]). Since
the most common use for this is to transmit thrown values for logging,
it makes sense to provide this helpful String instead of undefined.
(All this despite the fact it is a terrible idea to throw a function).

Based on:
jonpacker/destroy-circular#1

Fixes: #2

For serialized AssertionErrors, it is possible that the `actual`
and `expected` properties contain circular references, causing
a thrown Error when you serialize the result with JSON.stringify().

This destroys circular references on the copied Object. Replacing
them with a string (`[Circular]`).

Also, `JSON.stringify(function() {})` returns `undefined`, and
will drop any functions that are attached to an object it is serializing.
This mimics the latter behavior, but if a function is passed in as the top
level argument, it returns a string (`[Function: fnName]`). Since
the most common use for this is to transmit thrown values for logging,
it makes sense to provide this helpful String instead of `undefined`.
(All this despite the fact it is a terrible idea to throw a function).

Based on:
  jonpacker/destroy-circular#1

Fixes: #2
t.true(x.includes('message'));
const serialized = serialize(new Error('foo'));
const x = Object.keys(serialized);
t.not(x.indexOf('name'), -1);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like I found an AVA regression here.

x.includes stopped working for me

sindresorhus added a commit that referenced this pull request Nov 24, 2015
Incorporate destroy-circular logic. Fixes #2.
@sindresorhus sindresorhus merged commit ac8d60f into master Nov 24, 2015
@sindresorhus sindresorhus deleted the handle-circular-references branch November 24, 2015 08:42
@sindresorhus
Copy link
Owner

👍 Looks good.

jamestalmage added a commit to jamestalmage/ava that referenced this pull request Nov 24, 2015
We dropped it because it did not handle circular references.
That has since been fixed.

Reference:
  sindresorhus/serialize-error#3
jamestalmage added a commit to avajs/ava that referenced this pull request Nov 24, 2015
We dropped it because it did not handle circular references.
That has since been fixed.

Reference:
  sindresorhus/serialize-error#3
(cherry picked from commit 3e90385)
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.

2 participants