Skip to content
This repository has been archived by the owner on Oct 23, 2019. It is now read-only.

Custom replacer context #4

Closed
panta82 opened this issue Oct 1, 2013 · 5 comments
Closed

Custom replacer context #4

panta82 opened this issue Oct 1, 2013 · 5 comments

Comments

@panta82
Copy link

panta82 commented Oct 1, 2013

Hey, I'm using a custom replacer when stringifying objects such as dates and errors. After switching to circular-json, I noticed that my custom replacers weren't doing their job.

It seems the problem is here:

if (replacer) value = replacer(key, value);

This doesn't pass the usual context for JSON.stringify - instead of the object being stringified, 'this' references the global JSON object. Since I was doing something like:

if (this[key] instanceof Date) { /*do my custom toString() */ }

... my special converters weren't being called (a nasty silent fail). Suggested solution:

if (replacer) value = replacer.call(this, key, value);
@WebReflection
Copy link
Owner

you are right. It should be fixed now with tests included. Please check/test and close this ticket if it works as you expect. Thank you.

@WebReflection
Copy link
Owner

P.S. … in your very specific example, you don't need to check this[key] just value so …

if (value instanceof Date) return 'your thing';

That said, shared Dates objects across properties can be better packed via CircularJSON plus Date by default serialize into unserializable strings so eventually the problem would be in the parse() method (and I gonna check that too)

@WebReflection
Copy link
Owner

done, both fixe with the right context.

@panta82
Copy link
Author

panta82 commented Oct 1, 2013

No, the provided value is already converted by the default stringifier. In this particular case, instead of date object, I'm getting string (in the format eg: "2013-10-01T16:15:58.181Z").

Anyway, already fixed my local version and tested. Seems to be working ok. Closed.

@panta82 panta82 closed this as completed Oct 1, 2013
@WebReflection
Copy link
Owner

The reviver had same issue. Grab the latest and you should be ok

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

No branches or pull requests

2 participants