[11.x] Add non-static JsonResource wrapping

[SanderMuller]

Overview

This PR addresses an issue with setting JSON wrappers on resource collections or other general-purpose JSON resource classes where adjusting the wrapper for an AnonymousResourceCollection or other generic resource class would affect all instances in the same request or following tests. This PR introduces instance-specific JSON wrapping to JsonResource through a new $wrapper property, along with withWrapper() and withoutWrapper() methods. This change allows developers to set a wrapper for individual resources without impacting others globally.

Example of the Issue

Currently, modifying the wrapper for a resource collection applies globally within the same request or test:

// Setting a wrapper for a resource collection
AnonymousResourceCollection::wrap('posts');

// This affects ALL instances of PostResource in the same request
$postsCollection = PostResource::collection($posts); // Wrapped with 'posts'
$booksCollection = BookResource::collection($books); // Also wrapped with 'posts'

With this PR, the wrapper can be set on a per-instance basis:

// Per-instance wrapper configuration
$postsCollection = PostResource::collection($posts)->withWrapper('posts');
$booksCollection = BookResource::collection($books)->withWrapper('books');

// Result: Different wrappers for different instances
// $postsCollection is wrapped with 'posts'
// $booksCollection is wrapped with 'books'

Benefits

• Fine-Grained Control: Customize JSON wrappers for each resource instance, preventing unintended global side effects.
• More Flexible API Design: Easier to handle unique formatting requirements on a per-instance basis, particularly useful when working with collections.
• Consistency in Tests: Ensures that modifications to one test case's wrapper configuration won't carry over to others, making tests more reliable and isolated.

Why This Change Doesn't Break Existing Features

The default behavior remains the same because the new instance-specific $wrapper property respects the global $wrap setting unless overridden. Existing resource configurations and behaviors are not altered unless explicitly changed.

How It Makes Building Web Applications Easier

• Instance-Specific Adjustments: Developers can define how each resource or collection is wrapped, reducing complexity and avoiding the need for duplicate resource classes just to modify the wrapper behavior.
• Better Test Coverage: Isolated control over wrappers simplifies testing, allowing for more focused test cases without unintended side effects.
• Simpler Code: Reduces the need for workarounds or extra conditionals to handle resource-specific JSON wrapping, leading to cleaner and more maintainable code.

[SanderMuller]

This function would return null before this PR already in cases where ::withoutWrapping() was used

[danon]

@SanderMuller @taylorotwell This is a breaking change.

If non-static wrapper is not used in a class, it should return static ::$wrap. Otherwise, subsequent calls (after the instiantiation) to static methods ::withoutWrapping() or ::wrap() will be ignored for any class that extends JsonResource.

I can give you access to my repository, I have laravel at 11.33.2 and it works fine, but breaks at update to 11.34.0, and it breaks in places that touch wrapping.

Here's a test that illustrates it. It passes on 11.33.2, but fails on 11.34.0.

class JsonResourceTest extends TestCase
{
    #[Test]
    public function test(): void
    {
        $response = new ResourceCollection([
            new JsonResource(['foo' => 'bar']),                           // instantiation
        ]);
        JsonResource::withoutWrapping();                                  // subsequent call
        $this->assertSame([['foo' => 'bar']], $this->getData($response)); // breaking change at laravel 11.34.0
    }

    private function getData(ResourceCollection $collection): array
    {
        return $collection->toResponse($this->request())->getData(true);
    }

    private function request(): Request
    {
        return $this->createMock(Request::class);
    }
}

[SanderMuller]

@SanderMuller @taylorotwell This is a breaking change.

If non-static wrapper is not used in a class, it should return static ::$wrap. Otherwise, subsequent calls (after the instiantiation) to static methods ::withoutWrapping() or ::wrap() will be ignored for any class that extends JsonResource.

I can give you access to my repository, I have laravel at 11.33.2 and it works fine, but breaks at update to 11.34.0, and it breaks in places that touch wrapping.

Here's a test that illustrates it. It passes on 11.33.2, but fails on 11.34.0.

class JsonResourceTest extends TestCase
{
    #[Test]
    public function test(): void
    {
        $response = new ResourceCollection([
            new JsonResource(['foo' => 'bar']),                           // instantiation
        ]);
        JsonResource::withoutWrapping();                                  // subsequent call
        $this->assertSame([['foo' => 'bar']], $this->getData($response)); // breaking change at laravel 11.34.0
    }

    private function getData(ResourceCollection $collection): array
    {
        return $collection->toResponse($this->request())->getData(true);
    }

    private function request(): Request
    {
        return $this->createMock(Request::class);
    }
}

Static calls having impact after instantiation is a bit odd, but you're right that the wrapper changing for existing instances was how it worked before this PR. I don't think there's an easy fix for this, so this feature might have to be brought to 12.x or the current impact has to be deemed acceptable.

More Laravel alike test which exposes the impact:

    public function testResourcesCanSetWithoutWrappingAfterCreatingInstance()
    {
        Route::get('/', function () {
            $resource = new PostResource(new Post([
                'id' => 5,
                'title' => 'Test Title',
            ]));

            $resource::withoutWrapping();

            return $resource;
        });

        $response = $this->withoutExceptionHandling()->get(
            '/', ['Accept' => 'application/json']
        );

        $response->assertJson([
            'id' => 5,
            'title' => 'Test Title',
        ]);
    }

[danon]

@SanderMuller SanderMuller I agree that it's odd, but it was the only way to achieve the desired result with static wrapping. Here's an example project and code that uses that:

https://github.com/pradoslaw/coyote/blob/bbf2172b20d78c9fb47f75f8252540caaf9c3d5e/app/Http/Controllers/Forum/TopicController.php#L104

There is 10-30 more places in the codebase like that. I don't have the resources to update all of those places. Currently, I'm unable to update laravel to 11.34, because of that breaking change.

[SanderMuller]

@SanderMuller SanderMuller I agree that it's odd, but it was the only way to achieve the desired result with static wrapping. Here's an example project and code that uses that:

https://github.com/pradoslaw/coyote/blob/bbf2172b20d78c9fb47f75f8252540caaf9c3d5e/app/Http/Controllers/Forum/TopicController.php#L104

There is 10-30 more places in the codebase like that. I don't have the resources to update all of those places. Currently, I'm unable to update laravel to 11.34, because of that breaking change.

The sad thing is, this PR solves exactly the reason why the code you linked to came to be this odd.

[danon]

The sad thing is, this PR solves exactly the reason why the code you linked to came to be this odd.

Yea, the irony is evident to me too. I wish the fix came sooner, before the codebase was populated with the static approach.

I don't think there's an easy fix for this, so this feature might have to be brought to 12.x or the current impact has to be deemed acceptable.

I think the fix is quite easy - Currently, you return a different wrapper() if a class extends JsonResource, this is the breaking part - because it ignores whatever was set by static methods.

if ($this->resource instanceof JsonResource) {
    return $this->resource->wrapper;
}
return get_class($this->resource)::$wrap;

What can be done, is wrapper() method could check which "type" of wrapping is used (new or old). It can't simply check if ->wrapper is set, because you set it in constructor - so it's going to be set anyway. Maybe a simple static boolean $isStaticallyWrapped or something. Static methods could set it to true, while your non-static methods could set it to false. Then the wrapper() method could check it, and if it's true, return static ::$wrap.

Or in other words, this condition $this->resource instanceof JsonResource is not enough to decide whether to go with the non-static wrapper, because there are classes which extend JsonResource and don't use non-static wrappers. You need a stronger condition to use them IMHO.

[SanderMuller]

The sad thing is, this PR solves exactly the reason why the code you linked to came to be this odd.

Yea, the irony is evident to me too. I wish the fix came sooner, before the codebase was populated with the static approach.

I don't think there's an easy fix for this, so this feature might have to be brought to 12.x or the current impact has to be deemed acceptable.

I think the fix is quite easy - Currently, you return a different wrapper() if a class extends JsonResource, this is the breaking part - because it ignores whatever was set by static methods.

if ($this->resource instanceof JsonResource) {
    return $this->resource->wrapper;
}
return get_class($this->resource)::$wrap;

What can be done, is wrapper() method could check which "type" of wrapping is used (new or old). It can't simply check if ->wrapper is set, because you set it in constructor - so it's going to be set anyway. Maybe a simple static boolean $isStaticallyWrapped or something. Static methods could set it to true, while your non-static methods could set it to false. Then the wrapper() method could check it, and if it's true, return static ::$wrap.

Or in other words, this condition $this->resource instanceof JsonResource is not enough to decide whether the non-static should be used, because there are classes which extend JsonResource, but don't use non-static wrappers. You need a stronger condition to use them IMHO.

I'm looking into this suggestion, thanks

[SanderMuller]

@danon #53685 should fix your issue