search

sanity-io / GROQ

Specification for GROQ - The Query Language for JSON
https://groq.dev
MIT License
398 stars 15 forks source link

Added string function #38

Closed israelroldan closed 3 years ago

israelroldan commented 3 years ago

The string function has been proposed here: https://github.com/sanity-io/groq-js/pull/25

judofyr commented 3 years ago

Thanks for the PR!

I rebased the latest master and expanded the spec to be more explicit. Do you agree with this phrasing?

israelroldan commented 3 years ago

Thanks for tidying it up! The current implementation from sanity-io/groq-js#25 returns NULL_VALUE when the expression evaluates to null.

Would it perhaps make more sense to leave it like this? (instead of returning "null" as the new phrasing describes) (In this way you can still use coalesce to have a fallback if the expression evaluated to false: coalesce(string(somethingThatEvaluatesToNull), 'fallback') as opposed to string(coalesce(somethingThatEvaluatesToNull, 'fallback'))

The current implementation also delivers cleaner objects instead of { "somethingThatEvalutesToNull": "null" } which is consistent with GROQ's way of never returning null as a property value

judofyr commented 3 years ago

Good point. Another disadvantage of having string(null) return "null" is that string(string(foo)) becomes different from string(foo) (in the case where foo is an object).

israelroldan commented 3 years ago

Great, I'll fix that in the spec and re-run the build.

judofyr commented 3 years ago

Merged in a124ef264d10a90304952478124de79b2ef3054c! I also added a follow-up commit to make it handle dateTime as well.