Framework: Breaking change in ResourceCollection

Created on 23 Apr 2020  路  14Comments  路  Source: laravel/framework


  • Laravel Version: 7.7.1
  • PHP Version: 7.4.4
  • Database Driver & Version: mysql 8

Description:

cf https://github.com/laravel/framework/pull/32296#issuecomment-618246477
$this->collection->transform() throw an error Trying to get property 'resource' of non-object'

Works in 7.5.2, break in 7.6.0 and above

Have to replace
$this->collection->transform( by $this->collectResource($this->collection)->transform(

Steps To Reproduce:

Usage of $this->collection->transform based on https://medium.com/@dinotedesco/laravel-api-resources-what-if-you-want-to-manipulate-your-models-before-transformation-8982846ad22c

```

use Illuminate\Http\Resources\Json\ResourceCollection;
use App\User;

class IndexCollection extends ResourceCollection
{
public function toArray($request): array
{
return [
'data' => $this->collection->transform(static function (User $user) {
return [
'id' => $user->id,
'email' => $user->email,
];
}),
];
}
} ```

needs more info

Most helpful comment

Fixed in 7.8.0.

All 14 comments

Now that I see it like this, I'm not so sure this is a bug. We don't document transforming the dataset like this anywhere.

Maybe we should add extra guards in the code to help with developer experience, giving good exceptions?

@driesvints We know it's not the standard procedure, but sometimes people have to customize the payload a bit. Where a Resource::collection and ResourceCollection have to be different. In such cases, $this->collection->transform is a great help. But #32296 breaks that.

What are the values of the collection? How are you instantiating them?

I will explain with an example.

Say you have a Post model and you fetch a post using a PostResource whenever a GET request comes to /api/posts/<id>. Now you have added a lot of values and relationships to that, that are not really needed in the PostCollection. All your PostCollection needs, is an id and a title. In that case, it is necessary to customize the PostCollection a bit. Which can be done using $this->collection->transform.

But now that can't be done. #32296 will break a lot of these kind of applications.

What's preventing you from doing it the way that it's documented?

<?php

namespace App\Http\Resources;

use Illuminate\Http\Resources\Json\JsonResource;

class Post extends JsonResource
{
    /**
     * Transform the resource into an array.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return array
     */
    public function toArray($request)
    {
        return [
            'id' => $this->id,
            'title' => $this->title,
        ];
    }
}
use App\Http\Resources\ Post as PostResource;
use App\Post;

Route::get('/posts', function () {
    return PostResource::collection(Post::all());
});

I use it like that :

    public function __invoke(Request $request): IndexCollection
    {
        // some logic

        return new IndexCollection($pages);
    }

I prefere return a IndexCollection than an AnonymousResourceCollection and i can get all my select options metas in the Collection instead of adding it in controller with ->additional([])
I dont mind the change, it doesnt block me, just the error gave me a headache to find the cause.

Well, typically the collection uses the singular resource for the collection:

Typically, the $this->collection property of a resource collection is automatically populated with the result of mapping each item of the collection to its singular resource class. The singular resource class is assumed to be the collection's class name without the trailing Collection string.

I see your use case but I don't think we ever really supported that.

What's preventing you from doing it the way that it's documented?

Because some people (like me) keep PostResource reserved for single post and PostCollection for well, collection of posts, with some differences in the payload between the both.

I get that it's not the ideal way, but I was just explaining why people might want to use this kind of technique. It's pretty obvious. I agree with @GrahamCampbell, that there should be some kind of exception, if something is not supported.

Like @Shhu, it took me 2 whole days to figure out why that particular code was not working, only to find out later that a pull request was behind the errors.

This one took me a little while to figure out as well. I can't say for sure where I saw an example for the array style transform:

$queryResult
            ->getCollection()
            ->transform(function($pet) {
                return [
                    ...
                ];

But it did offer some benefits over the way transform is shown in the official documentation. Mainly, changing model attributes directly caused issues with some of my model mutators (eg reformatting dates to be strings, adding '$' to currency fields, etc). I wound up re-coding the transformers but I had to add extra attributes to my models that don't exist in the schema (which felt a little hacky).

Fixed in 7.8.0.

Was getting this error on Laravel v6.18. I guess I was doing the transformation based on this StackOverflow answer. I don't have enough reputation to comment a warning about it, but if anyone can, please do.

From the OP:

Steps To Reproduce:

Usage of $this->collection->transform based on https://medium.com/@dinotedesco/laravel-api-resources-what-if-you-want-to-manipulate-your-models-before-transformation-8982846ad22c

This article only ever transforms into resource objects. Not into plain arrays.

They even say that in the article:

Ok, if the resource collection doesn鈥檛 automagically convert the model into the corresponding resource what do we do?

We transform the collection of models into a collection of resources.

Fixed in 7.8.0.

@taylorotwell can you do the same for 6.x branch please?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

klimentLambevski picture klimentLambevski  路  3Comments

gabriellimo picture gabriellimo  路  3Comments

fideloper picture fideloper  路  3Comments

SachinAgarwal1337 picture SachinAgarwal1337  路  3Comments

felixsanz picture felixsanz  路  3Comments