Skip to content

Ensure toJson and toResponse call jsonSerialize #1012

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

clementbirkle
Copy link
Contributor

Description

I noticed that the toJson() and toResponse() methods do not use the jsonSerialize() method.

This can be problematic in certain use cases. For example, you might override jsonSerialize() to transform an empty array into an object to ensure consistent data structures on the frontend.

With this PR, the package will be more aligned with Laravel's behavior, which respects the jsonSerialize() method in similar scenarios.

Example

$data = new class extends Data
{
    public function __construct(
        public array $customFields = [],
    ) {}

    public function jsonSerialize(): array
    {
        $array = parent::jsonSerialize();

        // Ensure customFields is always an object
        $array['customFields'] = (object) $array['customFields'];

        return $array;
    }
};

// Current behavior:
json_encode($data);           // {"customFields":{}}
$data->toJson();              // {"customFields":[]}
$data->toResponse(request()); // {"customFields":[]}

// With this PR:
json_encode($data);           // {"customFields":{}}
$data->toJson();              // {"customFields":{}}
$data->toResponse(request()); // {"customFields":{}}

As shown, json_encode($data) and $data->toJson() currently produce inconsistent results. This PR fixes that.

Implementation Note

To make toResponse() respect jsonSerialize(), I introduced a new method: jsonSerializeWithTransformationContext(). This method may also be useful in other parts of the package, such as:

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

Successfully merging this pull request may close these issues.

1 participant