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

Need better handling of JSON keys #4

Open
Kit-p opened this issue Sep 28, 2022 · 2 comments
Open

Need better handling of JSON keys #4

Kit-p opened this issue Sep 28, 2022 · 2 comments

Comments

@Kit-p
Copy link

Kit-p commented Sep 28, 2022

Scenario:

{ "a.b": 1, "a": { "b": 2 } }

Description:

Since "a.b" is a valid key, the . must be escaped before appending to the path, in order to not be confused with "a"."b". A more robust way of dealing with this issue would be to quote the key, as you can also have things like whitespace in the key, or even an empty string as key. A reference approach at https://jqplay.org/s/7sO6XJmPdTZ where I recreated the scenarios using jq.

Code Reference:

https://github.com/apihero-run/json-hero-path/blob/b873f34546c3763b9c78987e504e86b34a295b9c/src/index.ts#L56

Possible Solution:

 return new JSONHeroPath(string.concat(`."${key}"`)); 

Related Issues:

None in this repository, but unfortunately caused one in triggerdotdev/jsonhero-web#98.


Feel free to let me know if there is a better way to solve this (or if my proposed solution will break something), otherwise I will start working on a PR.

@matt-aitken
Copy link
Member

matt-aitken commented Sep 28, 2022

Hi @Kit-p,

You can have .s in paths but they must be escaped with backslashes. The tests for this live here: https://github.com/apihero-run/json-hero-path/blob/b873f34546c3763b9c78987e504e86b34a295b9c/tests/parsing.test.ts#L19

I think your suggestion of quotes probably would have been better but I think this should work properly as long as paths are correctly generated.

Is this related to a JSON Hero issue? My guess would be that the JSON parser is not correctly encoding the paths with backslashes.

@Kit-p
Copy link
Author

Kit-p commented Sep 28, 2022

@matt-aitken Thank you for the information! Indeed if the key has the .s properly escaped before passing into the .child() function, then it works.

However, the one special case where an empty string is used as a key still causes a crash due to infinite recursion. I believe this issue should remain open.

An ugly fix (so we don't need to change anything here) would be to use \"\" for empty string in the parser for JSON Hero, but will need to deal with the fact that the json-source-map package using / as the pointer for empty string instead of /\"\".


Edit: I tried implementing the ugly fix mentioned above, can't get it to work because if component.keyName === '""', selecting the node will never be able to get the value in useSelectedInfo.

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

No branches or pull requests

2 participants