feat: support Date object serialization, make json more consistent with JSON.stringify #58
Conversation
There was a problem hiding this comment.
Could you update the README as well? E.g. here https://github.com/mathiasbynens/jsesc#jsescvalue-options
| 'json': true | ||
| } | ||
| ), | ||
| '"2020-04-03T15:34:00.000Z"' |
There was a problem hiding this comment.
This depends on how we want to deal with #57, Is this what you'd expect when passing a Date/Map/Set while using json: true? Or would you want to throw an exception instead?
There was a problem hiding this comment.
Either way this would need to be documented in the README under the json option.
There was a problem hiding this comment.
This is the behavior of toJSON. I'd like to add an additional flag in a subsequent PR (or, I suppose, in this PR) to let the user override that behavior independent of the JSON mode.
There was a problem hiding this comment.
Maybe we could include an interpret option that accepts values like expect-json, coerce-json, and javascript to switch between the options. expect-json would throw on non-JSON-supported values; coerce-json would invoke toJSON and delegate to JSON.stringify's behavior in other cases; javascript would do its best to serialize all (non-circular) values for use in a js context.
I don't need expect-json for my own work, but coerce-json would be extremely useful. I've also been wondering if we could introduce a replacer-type option for both transforming and extending the supported values - e.g. we need strict coerce-json semantics, but also want to support the undefined value. If you're amenable to that, I'll see what that would take.
There was a problem hiding this comment.
The way I've used jsesc is always very controlled, in that: if there's a Date object, a Map, a Set, or a BigInt somewhere in the input, and I am requesting JSON output via json: true, that's probably a mistake. I've only used jsesc in cases where I want JSON.parse(output) to produce an equivalent object again.
So I defer to you on this. If you have a use case for it, that's great!
For consistency with JSON.stringify. Backwards-compatible because JSON parsers would break on json: true output if Map or Set included in said output.
skeggse
changed the title
| @@ -134,7 +137,7 @@ const jsesc = (argument, options) => { | |||
| } | |||
|
|
|||
| if (!isString(argument)) { | |||
| if (isMap(argument)) { | |||
| if (!json && isMap(argument)) { | |||
There was a problem hiding this comment.
A potential solution for #57; Map and Set don't have toJSON methods - and even if they did, they probably wouldn't return the same type, so the expectation here is that we never process a Map or Set in a special way when targetting JSON.
|
I don't have use for this anymore, and I think I decided much of the problem I was encountering was just me using the wrong tool. |
Fixes #56, #57.