Open rbrother opened 5 years ago
That sounds reasonable, but I’ve not used the library in a while and don’t remember why I decided to return strings. I don’t have time to focus on this right now.
Unfortunately, I have to assume that changing this will break some clients. Can you take a look at the clients on GitHub that use this library (including my toy app https://github.com/deg/trilystro) and see what will break? Perhaps the safest idea will be to pass in a flag that specifies whether to return strings or keys, even though that’s kind of ugly.
I’ll be receptive to a PR, if you have time to do all this.
Thanks, David
I think I simply implemented the most general way possible (using strings instead of keywords, as there might be someone who prefers to use strings). I think there is a way to just pass a flag at initialization, and that flag could be checked at the time of converting JS objects to "Clojure".
Thanks for your quick replies! I will look at the codebase and see if I can implement a flag for retaining keywords in keys as suggested. I noticed the similar problem (though less common) is for map values that are keywords and also converted to strings. I have less problem with switching to use strings instead if keywords as the values of maps in my app, but for keys it's more important.
Another possibility instead of a flag would be to store :abc as ":abc" i.e. store a colon as sign of a keyword and then restore that string back to keyword. This would work well for both keys and values. Might of course break things as well, so better test it out.
Sounds like this is a more general question of what to do with the (lossy) conversion of clojure data structures to JSON and back again.
This does the conversion back:
Changing it to (js->clj ... :keywordize-keys true)
should do the trick.
What's worth notting is that within the library, the realtime database does keywordize-keys
, so follows a different convention than firestore, which is confusing.
I'd suggest a initialisation time flag be introduced to enable keywordizing behaviour, leaving the default as is. Perhaps a future major release could change the defaults to be consistent.
Will try to do a PR this week, life allowing :)
Thanks for excellent library, very useful and elegant! :-)
When I save data { :first "mike", :last "thompson"} with :firestore/add and then later read it with :firestore/get, the result is { "first" "mike", "last" "thompson"} ie. the keys have changed to strings.
I understand that firestore does not support keywords and needs to convert them to strings, but perhaps when reading data back from firestore your library could convert all keys in maps to keywords? I can do this myself, but you can consider if it might be ok to do it on lbrary level.