ljharb / json-stable-stringify

MIT License
55 stars 11 forks source link

Enhance `cmp` function args #4

Closed nikitaeverywhere closed 1 year ago

nikitaeverywhere commented 1 year ago

Hello! Thanks for the good package.

Within my use case, I need a custom sorting function, where having just keys and values of the things being compared is not enough. Specifically, I need to stable-sort "JSON comments" (properties ending with "!") before "JSON objects", and moreover sort objects before string values.

This is an example of a file with valid sort:

image

In order to perform this sort, in the cmp function, when the comment property "Prop!" is being compared with unrelated value "OtherProp", I need to know the comparison of "OtherProp" to "Prop" but not "Prop!".

I suggest a very tiny fix to your lib to pass the current object as the third argument to cmp:

image

// Pseudocode
jsonStringifyStable(obj, {
  cmp: ({ key: key1, value: value1 }, { key: key2, value: value2 }, obj) => {
    const cv1 = isComment(key1) ? obj[commentKey(key1)] : value1;
    const cv2 = isComment(key2) ? obj[commentKey(key2)] : value2;
    return /* Compare cv1 and cv2 here instead of v1 and v2 */
  }
})

Let me know what you think.

ljharb commented 1 year ago

This is a very strange commenting convention, tbh - altho a more reasonable one would be // foo for foo which does run into the same problem, so I'll ignore the convention :-)

This would allow the object to be mutated mid-comparison, which might really mess up the reliability of the algorithm. Any alternative API I can think of, however, would be much more complex, so I'm not really sure if this feature is worth the complexity it introduces. Do you have any alternative suggestions?

nikitaeverywhere commented 1 year ago

Thanks for your reply.

Regarding the convention, when you're limited to a pure JSON (not even JSONC) you have no way but to use property names for comments. I believe this is somewhat typical in language (translation) files, if no third-party services with fancy UIs are used, as well as in other scenarios.

As for the algorithm, following your reasoning, one can break it already (as it performs a recursive sorting and walks the entire object tree with its children, when sorting the parent I have direct access to mutated children values which I can change). So really, those who want to shoot themselves in the foot would do it regardless.

My current approach — I just Ctrl+C'd the entire (thank you - short) library code and added that argument, it worked like a charm.

If you prefer to avoid writes on the sorted object (which is, to note, quite reasonable), we can pass a getter-only readonly third argument, or even an «options» arg like { get } which will return just the value by the given object's property name, get(key: string): any.

ljharb commented 1 year ago

That seems like a viable alternative - just a straight get function that takes a key and returns obj[key]. I like it!

nikitaeverywhere commented 1 year ago

I may push a PR a bit later if you don't mind, dont you?

ljharb commented 1 year ago

@nikitaeverywhere i don't mind at all :-) sorry for the late reply! in the future if there's a help wanted label you can just make a PR.

nikitaeverywhere commented 1 year ago

@ljharb no worries. PR is ready #5