-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[12.x] Fix: Add $forceWrap property to JsonResource for consistent API response #56724
#56736
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
Conversation
$forceWrap property to JsonResource for consistent API response #56724$forceWrap property to JsonResource for consistent API response #56724
|
This fix will be backported to 11.x right ? |
|
@challapradyumna I think its possible as it would be the same change. |
|
@achrafAa the opt-in should be default from a logical perspective. Did you contact the owner of that line $this->wrapper() && ! array_key_exists($this->wrapper(), $data);to ask for hes/her reasoning on this? |
|
@macropay-solutions, logically, consistent wrapping might seem like the better default—especially to ensure predictable API responses regardless of internal data structure. That said, I did some digging and saw that this particular behavior comes from this line authored by Taylor back in 2017: I’d be interested in understanding the original intent behind this condition. My current interpretation is that it was an optimization to avoid redundant wrapping when the resource data already includes a data key. But in practice, as shown in the example, it can lead to inconsistent API responses when models happen to have a data attribute. @taylorotwell – would you be open to sharing your original thinking behind this behavior? Do you think the forceWrap opt-in approach is an acceptable compromise, or would it make sense to revisit the default behavior? |
|
@achrafAa maybe to avoid infinite loops or... |
|
Why don’t we simply remove the check? ! array_key_exists($this->wrapper(), $data) |
Hello @achrafAa |
|
@AhmedAlaa4611 we want to avoid breaking changes, that check was put for a certain X raison, thus deleting it may cause those scenarios to break app flow. |
What about reverting this and removing the check from master? To be honest, it feels inconsistent that the framework sometimes applies wrapping and other times doesn’t. If consistent wrapping is needed, the It’s unfortunate to introduce this kind of inconsistency, especially since Taylor declined to document it. |
|
@AhmedAlaa4611 Not sure about removing it from the master branch, but for me, avoiding breaking changes is the way to go — especially since this Resource is widely used by packages and businesses. The refusal of a documentation PR doesn’t necessarily mean Taylor disagrees. It could also just mean avoiding fluff or overly opinionated API-style docs. That said, feel free to create a PR to address your concern and add documentation from your point of view, as long as it doesn’t introduce breaking changes. I’ll be happy to support it if it makes sense to me. 🙏 At the end of the day, it’s all about stability over opinionated decisions. |
Fix: Add
$forceWrapproperty to JsonResource for consistent API response wrappingProblem
check #56724
Laravel JsonResource wrapping behavior is inconsistent when the resource data contains a key that matches the
$wrapproperty (usually'data'). This causes unpredictable API responses depending on the internal structure of the model.Current Behavior (Bug)
Root Cause
In
ResourceResponse::haveDefaultWrapperAndDataIsUnwrapped(), wrapping is skipped if the resource array already contains a top-level key equal to$wrap:Solution
Added an opt-in
$forceWrapstatic property toJsonResourcethat forces wrapping even when the wrapper key exists in the resource data.Changes Made
1. Add
$forceWrapproperty to JsonResource2. Update ResourceResponse to check
$forceWrapUsage
Results
Benefits
✅ Predictable API Responses - Response shape is no longer dependent on model field names
✅ Backward Compatible - Default behavior unchanged, purely opt-in
✅ Explicit Developer Intent - Clear declaration of wrapping behavior
✅ Supports Modern API Patterns - Enables JSON:API, GraphQL-style APIs with reserved keys
Test Coverage
Added simple, direct tests:
testResourceSkipsWrappingWhenDataKeyExists- Demonstrates the original bugtestResourceWrapsWhenDataKeyDoesNotExist- Verifies normal behaviortestResourceForceWrapOverridesDataKeyCheck- Verifies the fix worksTests directly call
$resource->toResponse()and verify JSON output - no complex route setup needed.All existing tests pass (76/76), confirming no breaking changes.
Impact
This fixes a long-standing inconsistency in Laravel resource responses that has confused developers and created brittle API contracts. The solution follows Laravel's design philosophy of being explicit over implicit while maintaining full backward compatibility.