[8.x] Support JSON encoding Stringable

[reinink]

Currently if you JSON encode a Stringable instance, you'll get an empty object ({}). This PR updates the Stringable class to implement JsonSerializable, so that it's properly converted to a string when JSON encoded.

json_encode(Str::of('foo'))

// Before: {}
// After: "foo"

I ran into this when returning JSON from a controller, and the Stringable was an empty object instead of a string.

[GrahamCampbell]

Isn't this a breaking change, for someone already serialising it?

[reinink]

@GrahamCampbell Hmm, maybe? I don't see how though. Can you give an example of what you're thinking?

If they are already JSON serializing it, they'll be getting an empty object ({}), which obviously isn't what they want, nor is it useful in any way.

[crynobone]

There's a small breaking change if you return a Stringable response directly, so it was merged only on 9.x branch. #35680

It would be nice if we cast it directly on Response object to string so it can be merged as none breaking change.

[taylorotwell]

Even though it is a breaking change - it seems silly to depend on an empty object being returned when you return a string that isn't empty?

[GrahamCampbell]

Yeh, this is probably fine, on balance.

[reinink]

Thanks for jumping in everyone.

First off, sorry I didn't catch the 9.x change. I should have done a search prior to submitting this.

So to be clear, the risk here is that someone may be returning Str::of('whatever') from their controller, and the current behaviour will result in whatever being sent to the browser, but with my change, "whatever" will be sent instead. That is a legitimate breaking change, although quite unlikely that it will actually cause anyone issues.

On the other hand, this JSON encoding "bug" is quite likely to cause issues, and I'd love to see this fixed in 8.x, if at all possible. What I propose is that, in addition to this change, we also update the Illuminate/Routing/Router@toResponse method to check if the response is an instance of Stringable, and return a plain string if it is. This solves both use cases.

I already have this working, and can update this PR if you'd like. 👍

[crynobone]

@reinink my comment just to elaborate on the current state, I also looking at porting back the PR to Laravel 8 when Taylor considering to delay Laravel 9 late last week and haven't decided on the best way to handle BC on response (if we want to handle it). Few things in my mind right now:

• Directly cast Illuminate\Support\Stringable on response.
• Look into integrating symfony/polyfill-php80 and implements the new Stringable interface, but that could cause breaking change if Arrayable/JsonSerializable doesn't response the same value as __toString()

[reinink]

@crynobone Understood, thanks. I think directly casting the Stringable is a really simple way forward here, so that's what I'd recommend.

I've updated this PR to:

1. Resolve conflicts.
2. Update the Router (as proposed above).
3. Use __toString() instead (as requested by @driesvints).

[taylorotwell]

@reinink sounds good

[reinink]

@taylorotwell Awesome. The PR should be ready to go. 👍

[GrahamCampbell]

Excellent work, everyone!

[JayBizzle]

Just thought I would mention that this update broke our application.

We have some middleware that injects a <script> just before the </body tag when certain conditions are met. I have simplified this example, but it looks along the lines of this...

public function handle($request, Closure $next, $guard = null)
{
    $response = $next($request); 

    $response->setContent(
        Str::of($response->getContent())->replaceLast('</body>', '<script>/* snipped */</script></body>')
    );

    return $response;
}

What was happening was the raw HTML was being output to the browser as quoted text.

We have resolved the issue by changing the to the following...

public function handle($request, Closure $next, $guard = null)
{
    $response = $next($request); 

    $response->setContent(
        str_replace('</body>', '<script>/* snipped */</script></body>', $response->getContent())
    );

    return $response;
}

Perhaps this is a very edge case, but just thought it was worth mentioning.

Thanks