-
Notifications
You must be signed in to change notification settings - Fork 11.6k
[13.x] Change InteractsWithData::string()
to return native string
#56866
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
base: master
Are you sure you want to change the base?
[13.x] Change InteractsWithData::string()
to return native string
#56866
Conversation
Can you rebase with latest from |
done |
…nto distinct-request-string-and-str
I honestly am not sure the juice is worth the squeeze here. It's a breaking change that fixes a relatively minor inconsistency but is just one more thing for developers to think about. If we can automate it via Rector I'm more open to it I think. What is the best way for us to do that in the core without requiring any extra packages? |
@TomasVotruba @rectorphp what would be a good strategy to get Laravel Rectorised? :) |
@taylorotwell we already maintain a set of Rector rules for Laravel upgrades at https://github.com/driftingly/rector-laravel It wouldn't be a problem to host a new L13 rule and then point to it in the official docs. Happy to write the rule as well 👍 |
This PR modifies the
InteractsWithData::string()
(and thus, a popularFormRequest::string()
) method to return a native PHP string instead of aStringable
instance, making it consistent withConfig\Repository::string()
and avoiding unnecessary object instantiation when developers only need a string value.Good news:
InteractsWithData::str
will returnStringable
what is sort of inline with the\Illuminate\Support\Str
class name.Motivation
Currently, both
string()
andstr()
methods inInteractsWithData
returnStringable
instances, which creates:Config\Repository::string()
which returns native stringsStringable
methodsstring()
andstr()
methodsWith these changes,
\Illuminate\Config\Repository::string
and\Illuminate\Support\Traits\InteractsWithData::string
have the same behaviour and similar code:While this can be one of the most annoying upgrade steps, I think it: