Add an option to throw on duplicate object keys
Why this feature is better to be included in comment-json? Please describe the scenario In most libraries, duplicate JSON object keys are generally not considered as an instant exception during parsing, and the later value just silently overwrite the former value with the same key. comment-json does the same thing, however this common practice causes surprising behavior here because object key also links to comments, resulting to inconsistent handling of duplicate keys:
const dup = `{
"a": 0, // comment on first a
"b": 1, // some other content
"a": 2
}`
stringify(parse(dup), null, 2)
// {
// "a": 2, // comment on first a
// "b": 1 // some other content
// }
So it might be necessary to treat duplicate keys as a SytaxError here for the ambiguity (which is not possible for compatibility) or add an allowDuplicateKey option in parse so the user could choose to get a warning on problematic JSON data, since it's really hard to detect duplicate keys outside of the parsing cycle.
Are you willing to submit a pull request to implement this rule? Yeah I'm very happy to implement this along side its docs, tests, .etc
Thanks. Could you create a PR for this?
- Suggest to override the parameters of the
parsemethod for backward compatibility
// Pseudo
parse(string, reviver, removeComments: boolean = false)
parse(string, reviver, {removeComments: boolean = false, allowDuplicateKeys: boolean = true} = {})
What do you think?
-
Use a custom error type instead of Syntax Error, since it actually is not a syntax error.
-
Please follow the minimum change principle
-
Please update the index.d.ts and tests
parse(string, reviver, {removeComments: boolean = false, allowDuplicateKeys: boolean = true} = {})
I'd use something like throwOnDuplicateKey instead of allowDuplicateKeys, so migrating to a new Version can simply be done by:
parse(s, r, false);
//
to
parse(s, r, { removeComments: false });
Due to allowDuplicateKeys being undefined, it would be falsy and may fall-back to false, which is a different behaviour. Inverting that logic to throwOnDuplicateKey would mitigate these kinds of migration issues.
But that only applies if throwing on a duplicate key may not be considered the default in future (breaking) versions.