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

Pass through JSON.stringify optional arguments #3

Merged
merged 1 commit into from
Apr 23, 2014

Conversation

chahu
Copy link
Contributor

@chahu chahu commented Apr 23, 2014

JSON.stringify takes two optional arguments. This change simply allows passing these arguments through Resurrect's stringify to the underlying call to JSON.stringify. In particular this is useful if certain Object properties should not be included in the serialized JSON (this can be accomplished with the replacer arg).

JSON.stringify takes two optional arguments.  This change simply allows passing these arguments through Resurrect's stringify to the underlying call to JSON.stringify.  In particular this is useful if certain Object properties should not be included in the serialized JSON (this can be accomplished with the replacer arg).
@skeeto skeeto merged commit 93e0800 into skeeto:master Apr 23, 2014
@skeeto
Copy link
Owner

skeeto commented Apr 23, 2014

I'm not sure the space argument has any value here, but I could see how the replacer might be useful.

@chahu
Copy link
Contributor Author

chahu commented Apr 23, 2014

Agreed. I needed the replacer. I figured I'd add both optional arguments for the sake of consistency. I don't have any concerns if you remove the space argument.

@skeeto
Copy link
Owner

skeeto commented Apr 25, 2014

I just added two commits, 76f78f8 and 7ce6ad8, that silently wrap the provided replacer so that it doesn't interfere with the intrusive ResurrectJS properties. I don't want the user to be concerned with those details, pretending that this is basically just a smarter JSON.stringify(). I want to know if you think this makes sense.

@chahu
Copy link
Contributor Author

chahu commented Apr 25, 2014

Makes perfect sense. Thanks.

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