Closed Zamfi99 closed 1 month ago
@Zamfi99 Thank you! This looks like a good idea, especially fixing the boolean examples you gave. We'd like to merge this soon and mostly need to consider possible API version updates to account for the changes in data representation.
@Zamfi99 I wanted to add some more tests for other scalar types, so I pushed a couple of changes to your PR fork: https://github.com/Zamfi99/vip-block-data-api/pull/1. PRing to a fork is not a typical workflow for me, but I think it makes sense here. If you can approve those changes to your fork, we're all set to merge in this PR. If it takes a bit longer, I'll probably migrate these changes to a new branch and merge that instead.
Thank you for your time and improvements to the plugin!
@Zamfi99 Merging this in to planned-release/1.3.0
, and planning to address your other open PRs (https://github.com/Automattic/vip-block-data-api/pull/65, https://github.com/Automattic/vip-block-data-api/pull/67) before making a release next week. Thanks a ton for your time so far!
Description
The current implementation of
strval
results in the loss of type information for non-string values. This transformation can lead to unexpected behavior in front-end (FE) applications and may disrupt existing contracts.The usage of strval on non-string values results in the following conversions:
strval(false)
results in""
strval(true)
results in"1"
strval(5)
results in"5"
Such conversions can alter the original types, potentially causing unforeseen issues in FE applications that rely on precise data types.
This PR introduces a change to encode non-string types as JSON strings. This approach ensures that the original types can be accurately decoded and maintained in FE applications, thereby preserving the expected behavior and integrity of existing contracts.
Steps to Test
composer run test
.