Normalizer: float precision

[boesing]

Hey there,

as we already had issues in valinor regarding float and int strict comparison, I wanted to bring this topic up.

I am not sure how different languages handle this, but I would assume that int can be a float as well, however, to prevent that, we sometimes used JSON_PRESERVE_ZERO_FRACTION.

Valinors JsonNormalizer is just casting floats to string which drops the zero fraction and converts floats to int in case they do not have decimals.

Should we either add a way to enable this behavior or is this ignored on purpose?
Current workaround is to use the array normalizer and json_encode on our own in combination with JSON_PRESERVE_ZERO_FRACTION.

[romm]

Hi @boesing, sorry I took that long to come back to you.

I'm wondering if we should not just force JSON_PRESERVE_ZERO_FRACTION in:

Valinor/src/Normalizer/Formatter/JsonFormatter.php

Line 36 in 3f1d86f

$this->write(json_encode($value, JSON_THROW_ON_ERROR));

Do you think it would make sense as a default behavior?

[boesing]

[...] sorry I took that long to come back to you.

All good, this is how OSS works. No demanding from my side 🤙🏼

[...] Do you think it would make sense as a default behavior?

For me that would work, but no clue if other projects using valinor do actually depend on the fact that the zero fraction is dropped. 😅

I am actually trying to implement an optional argument to Format::json where one can pass flags as int-mask-of of supported JSON_* constants.

Just need to find out how to pass these to the JsonNormalizer afterwards but should be doable. But kinda seems to be problematic due to the way how the normalizer is retrieved from the container as there is no "factory" logic. I'll see if I am able to find a way tho.

Thoughts?

[romm]

All good, this is how OSS works. No demanding from my side 🤙🏼

🤗

I am actually trying to implement an optional argument to Format::json where one can pass flags as int-mask-of of supported JSON_* constants.

Just need to find out how to pass these to the JsonNormalizer afterwards but should be doable. But kinda seems to be problematic due to the way how the normalizer is retrieved from the container as there is no "factory" logic. I'll see if I am able to find a way tho.

Thoughts?

The container is completely closed to any modification to reduce the maintenance burden. 😉

IMO, some of JSON_* consts make no sense in Valinor's scope, such as JSON_FORCE_OBJECT or JSON_PARTIAL_OUTPUT_ON_ERROR. However, if we accept only a white list of JSON_* constants, I may give access to a new JsonNormalizer::withOptions(int $optionsBitmask). Something like:

(new \CuyZ\Valinor\MapperBuilder())
    ->normalizer(\CuyZ\Valinor\Normalizer\Format::json())
    ->withOptions(JSON_PRESERVE_ZERO_FRACTION | JSON_UNESCAPED_SLASHES)
    ->normalize(/* … */);

This is the white list that makes sense to me, WDYT?

• ❌ JSON_FORCE_OBJECT
• ✅ JSON_HEX_QUOT
• ✅ JSON_HEX_TAG
• ✅ JSON_HEX_AMP
• ✅ JSON_HEX_APOS
• ✅ JSON_INVALID_UTF8_IGNORE
• ✅ JSON_INVALID_UTF8_SUBSTITUTE
• ✅ JSON_NUMERIC_CHECK
• ❌ JSON_PARTIAL_OUTPUT_ON_ERROR
• ✅ JSON_PRESERVE_ZERO_FRACTION
• ❓ JSON_PRETTY_PRINT —> would require more work, could be an option for later
• ✅ JSON_UNESCAPED_LINE_TERMINATORS
• ✅ JSON_UNESCAPED_SLASHES
• ✅ JSON_UNESCAPED_UNICODE
• ❗ JSON_THROW_ON_ERROR —> is forced by Valinor

[boesing]

Sounds reasonable to me. I can prepare a PR 👍🏻

[romm]

Would be wonderful!

[boesing]

Hm, I realize that there is only json_encode used for scalar values which are not bool.
Its not used for object properties.
I created this example from nette/utils issue (just as I was searching for a usage of JSON_HEX_AMP):
https://3v4l.org/snKuY

So it seems that it should also use json_encode for object properties. Do you think that should be supported by Valinor?
I'd be fine with not supporting it as I do not have a use-case for this but just wanted to bounce that with you.

[romm]

Just saw your comment although I already posted code-review of #512. 😬

When you write about "object properties" I guess what you mean is "keys"? If that's it, then yes I believe the JSON formatter should stick to the native behavior. This can be done here:

https://github.com/CuyZ/Valinor/blob/4c62d87a68a7de4e60d070cb7298a19fd7ebad5a/src/Normalizer/Formatter/JsonFormatter.php#L62C13-L62C53

Will you have time to update #512? If not I'll take care of it. 😉

[boesing]

I might find some time next weekend, but not sure yet. Feel free to take over in case you find some spare time before I do ☺️