Skip to content
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

[1.x] Fix resolving response arrayable properties #631

Open
wants to merge 4 commits into
base: 1.x
Choose a base branch
from

Conversation

VicGUTT
Copy link

@VicGUTT VicGUTT commented May 29, 2024

Prior to #620, property instances were resolved before handling arrayable/array values when sending a response.

Consider the following Inertia response dump:

AsArrayable
use Illuminate\Contracts\Support\Arrayable;

class AsArrayable implements Arrayable {
    public function __construct(private array $data) {}

    public static function make(array $data): static {
        return new static($data);
    }

    public function toArray(): array {
        return $this->data;
    }
}
use Inertia\Inertia;
use Illuminate\Http\Request;

$request = Request::create('/auth', 'GET');

$response = Inertia::render('auth', [
    'user' => fn () => [
        'full_name' => 'Victor Gutt',
        'organizations' => AsArrayable::make([
            fn () => [
                'name' => 'Transl.me',
            ],
        ]),
    ],
]);

$response = $response->toResponse($request);

dd(
    $response->original->getData()['page']['props'],
);

Before this fix, the dump would be similar to:

Inertia\Tests\ResponseTest
array:1 [
  "user" => array:2 [
    "full_name" => "Victor Gutt"
    "organizations" => Inertia\Tests\Stubs\AsArrayable {#1291
      #data: array:1 [
        0 => Closure() {#1277
          class: "Inertia\Tests\ResponseTest"
          this: Inertia\Tests\ResponseTest {#173 …}
        }
      ]
    }
  ]
]

As we can see, nested (user.organizations) arrayable values are no longer resolved.

After this fix, the dump is similar to:

Inertia\Tests\ResponseTest
array:1 [
  "user" => array:2 [
    "full_name" => "Victor Gutt"
    "organizations" => array:1 [
      0 => array:1 [
        "name" => "Transl.me"
      ]
    ]
  ]
]

This PR aims to bring back the behaviour prior to #620.

@driesvints driesvints changed the title [1.x] - Fix resolving response arrayable properties [1.x] Fix resolving response arrayable properties May 29, 2024
@VicGUTT
Copy link
Author

VicGUTT commented Jun 13, 2024

@reinink Resolved any conflicts that arose after #641. This PR is now up to date with the 1.x branch.

As a result, this PR only contributes the test_nested_arrayable_prop_response test case in tests/ResponseTest.php.
This test case is kept in place to ensure future changes does not break the evaluation of nested "Arrayable" instances as was previously the case with 620.

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