-
Notifications
You must be signed in to change notification settings - Fork 11k
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
[11.x] Introduce Support\Url
and Support\UrlQueryParameters
classes for easier URL handling
#53370
base: 11.x
Are you sure you want to change the base?
Conversation
I think this would be a great addition! good job! |
Really great addition! I've used the |
Love this, superb utility. |
Wow amazing, you have inspired me to do some new work |
$url = url()->parse('https://example.com/path/to/resource?foo=bar&baz=qux'); I like this method 😉 |
Would it be out of the scope to add this functionality to the Otherwise it is macroable, those who do want it could just add it ourselves. Great work Steve!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice !
What is the L11 standard practices for tests?
Would be nice if it also supported URI templates but that's probably a little out of scope |
Nice addition, thanks! I think it would be more powerful if $url->query()->get('filters'); // Or $url->query('filters')
$url->query()->has('filters');
$url->query()->count();
$url->query()->only('sort');
// ... Also // https://laravel.com/docs/11.x/collections
$url->path()->first(); // docs So we can have |
It would be nice to have documentation for this! |
Probably be best to wait until it gets merged before putting in the effort to document it. |
I've wanted a URL helper in Laravel for a while now. Glad to see this PR. I would love to see the ability to manipulate the URL and also a way for the I've always disliked the Laravel's At times I've needed to add AND remove query parameters from the current URL. I've love to see: $url = Request::toUrl()->withQuery([...])->withoutQuery([...]); In its current form, this helper does not seem to be able to manipulate with the URL in anyway. I always imagined the Laravel URL class would offer similar functionality to |
That would be awesome! 😍
Wouldn't that be $url = Request::toUrl()->withQueryParams([...])->withoutQueryParams([...]); as Going with Laravel's convention of
|
Other possible features:
What kind of validation do you want to have?
Helpers:
Which standards should be supported? |
Love this wrapper around $url = Url::parse('https://example.com?foo=bar');
$foo = $url->query->foo; // bar |
The suggestions here are getting quite wild. This started out as a thin wrapper around The first step should be to define the goal of this tool. If this is to become a complete and versatile URL handler, we should probably take a look at something like https://github.com/TRowbotham/URL-Parser or at least https://github.com/spatie/url instead of reinventing them by adding ideas one by one. Either way, if a "polished" URL tooling is added, it probably should not use Personally I think if such a tool is added, it should at least prove it's usefulness in the framework code itself. It should be something that makes the existing URL handling cleaner, shouldn't it? Btw there is a similar RFC on PHP itself: https://wiki.php.net/rfc/url_parsing_api |
My suggestions mostly weren't specifically meant for version 1, more as a hint to maybe develop version 1 with the future scope in mind. |
Sure @shaedrich I did not mean that any of the suggestions are invalid or aggressive, just that the scope is blowing up more than the initial architecture would be appropriate for. So I suggested that it would be useful to stop and think about the purpose. Either it's a sugar around |
Hey @stevebauman - dig this overall! Let's try to implement some of @timacdonald's feedback as a lightweight modification to start and then I think we could get this merged. Please mark as ready for review when the requested changes have been made. |
@taylorotwell Ok great, will do! |
Ok all set! Most notably, I've updated the Developers can change the object (and any of the properties) which will automatically be reflected when the $url = Url::parse('https://example.com/path/to/resource?foo=bar&baz=qux');
$url->path = '/some/other/path';
$url->query->set('foo', 'baz');
$url->query->forget('baz');
(string) $url; "https://example.com/some/other/path?foo=baz" Let me know your thoughts! 👍 |
Co-authored-by: Tim MacDonald <[email protected]>
Co-authored-by: Tim MacDonald <[email protected]>
@stevebauman I understand this is quite late in the process, but I just noticed that some bits of code work with the Uri class that is currently in an indirect dependency via Guzzle. We have: use GuzzleHttp\Psr7\Uri;
use GuzzleHttp\Psr7\Query;
// or Utils::uriFor($uriString);
$x = new Uri('https://example.com/path#fragment');
echo $x->getScheme();
Query::parse('m.y%20key=abc') What do you think about using these parsers instead of the PHP ones? Especially the Query one solves a lot of PHP's quirks, bet there are some patches for the URL parser as well. |
@tontonsb Oh, don't worry. I didn't understand that any differently. I just expected Taylor to chime in at some point (which he eventually did) and ask for a lightweight implementation for the first version so it can be merged swiftly. Using these packages does sound interesting for subsequent versions, especially, since we won't have to maintain that much code on our own then. |
@tontonsb Ou that's hard to say. I'm not sure. I'm not personally against it -- just not sure about depending on an indirect dependency... @timacdonald What are your thoughts on this? |
I don't have strong opinions there. As I mentioned in my initial comment...
Can just I think as long as it does the thing it is fine. We can always refactor to use the underlying package after. Either way, I'd wait for Taylor's feedback before making any changes. |
Description
This PR introduces two new classes in the
Support
namespace to provide a more convenient way to work with URLs with an object-oriented representation using PHP's nativeparse_url
andparse_str
functions:Illuminate\Support\Url
Illuminate\Support\UrlQueryParameters
I copy these same classes over from project to project as I usually end up needing them. I figured others may find these classes a more (IDE) friendly replacement to the existing
parse_url
andparse_str
functions.Example
Let me know your thoughts! 👍 No hard feelings on closure ❤️