Skip to content
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

Open
wants to merge 21 commits into
base: 11.x
Choose a base branch
from

Conversation

stevebauman
Copy link
Contributor

@stevebauman stevebauman commented Nov 1, 2024

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 native parse_url and parse_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 and parse_str functions.

Example

use Illuminate\Support\Url;

// Parse a URL string
$url = Url::parse('https://example.com/path/to/resource?foo=bar&baz=qux');

// Access URL components
$scheme = $url->scheme; // 'https'
$host = $url->host; // 'example.com'
$path = $url->path; // '/path/to/resource'

// Work with query parameters
$queryParams = $url->query; // UrlQueryParameters
$foo = $queryParams->get('foo'); // 'bar'
$baz = $queryParams->get('baz'); // 'qux'
$default = $queryParams->get('bar', 'default'); // 'default'

// Check if a query parameter exists
$hasBar = $queryParams->has('bar'); // true

// Set a query parameter
$queryParams->set('foo', 'baz');

// Convert to array
$urlArray = $url->toArray(); // ['foo' => 'bar', 'baz' => 'qux']

Let me know your thoughts! 👍 No hard feelings on closure ❤️

@niladam
Copy link

niladam commented Nov 1, 2024

I think this would be a great addition!

good job!

@heychazza
Copy link

Really great addition! I've used the parse_url call for a project and this would've made life a lot easier

@danmatthews
Copy link
Contributor

Love this, superb utility.

@Ali-Hassan-Ali
Copy link

Wow amazing, you have inspired me to do some new work

@Ali-Hassan-Ali
Copy link

$url = url()->parse('https://example.com/path/to/resource?foo=bar&baz=qux');

I like this method 😉

@joshmanders
Copy link
Contributor

Would it be out of the scope to add this functionality to the UrlGenerator under the parse() method like @Ali-Hassan-Ali says below?

Otherwise it is macroable, those who do want it could just add it ourselves.

Great work Steve!

$url = url()->parse('https://example.com/path/to/resource?foo=bar&baz=qux');

I like this method 😉

src/Illuminate/Support/Url.php Outdated Show resolved Hide resolved
src/Illuminate/Support/Url.php Outdated Show resolved Hide resolved
Copy link
Contributor

@raphaelcangucu raphaelcangucu left a 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?

@shaedrich
Copy link

Would be nice if it also supported URI templates but that's probably a little out of scope

@hafezdivandari
Copy link
Contributor

hafezdivandari commented Nov 2, 2024

Nice addition, thanks! I think it would be more powerful if UrlQueryParameters extend Illuminate\Support\Collection:

$url->query()->get('filters'); // Or $url->query('filters')
$url->query()->has('filters');
$url->query()->count();
$url->query()->only('sort');
// ...

Also fragment can have the same functionality, and having path may be useful too:

// https://laravel.com/docs/11.x/collections

$url->path()->first(); // docs

So we can have UrlComponent abstract class that extends Collection, and UrlQueryParameters, UrlFragments, and UrlPath that implement this class.

@ezequidias
Copy link

It would be nice to have documentation for this!
https://github.com/laravel/docs/pulls

@joshmanders
Copy link
Contributor

@ezequidias

It would be nice to have documentation for this! https://github.com/laravel/docs/pulls

Probably be best to wait until it gets merged before putting in the effort to document it.

@timacdonald
Copy link
Member

timacdonald commented Nov 3, 2024

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 Request instance to return a Url instance.

I've always disliked the Laravel's Request object only allows returning the URL as a string, e.g., Request::url(), Request::fullUrl(), Request::fullUrlWithQuery([...]), Request::fullUrlWithoutQuery([...]). Having these return a string means only one operation can be performed on it before you need to parse it again.

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 league\uri, with a more Laravel-style API.

@shaedrich
Copy link

shaedrich commented Nov 4, 2024

I would love to see the ability to manipulate the URL and also a way for the Request instance to return a Url instance.

That would be awesome! 😍

$url = Request::toUrl()->withQuery([...])->withoutQuery([...]);

Wouldn't that be

$url = Request::toUrl()->withQueryParams([...])->withoutQueryParams([...]);

as withoutQuery() would imply "without the entire query string/without the entirety of the query parameters", therefore with* chained with without* sounds like a contradiction.

Going with Laravel's convention of only() and except() like @hafezdivandari suggested probably would be more intuitive, wouldn't it?

$url->query()->only('sort');

@shaedrich
Copy link

shaedrich commented Nov 4, 2024

Other possible features:

  • host vs hostname vs authority vs origin
  • pathinfo vs href
  • Returning the host as object which implements an interface (the implementations can be hostname, IPv4, IPv6)
  • path could also be a Stringable Collection (optionally dirname, filename, and extension if present`)
  • scheme and port could be enum-like wrapper objects (isDefault(), isCustom())
  • ArrayAccess for query
  • data urls

What kind of validation do you want to have?

  • IP is valid
  • port is numeric and within range
  • string components only contain valid characters
  • URL max length (depends on browser)
  • query string max length (depends on server)

Helpers:

  • usesTls()/isSecure()
  • isOpaque()
  • isAbsolute(), isRelative()

Which standards should be supported?

@dennisprudlo
Copy link
Contributor

dennisprudlo commented Nov 4, 2024

Love this wrapper around parse_url. I agree that modification methods would make a nice addition as well (being able to set a specific query parameter to another value). Using a collection for the query parameters would allow for that as already proposed.

$url = Url::parse('https://example.com?foo=bar');
$foo = $url->query->foo; // bar

@tontonsb
Copy link
Contributor

tontonsb commented Nov 10, 2024

The suggestions here are getting quite wild. This started out as a thin wrapper around parse_url and parse_str but people are adding and adding their wishes and ideas.

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 parse_url and it certainly should not use parse_str. Whatever is returned from parse_url probably shouldn't be called URLSearchParams as that would make people expect it to behave like standards say URLSearchParams should.

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

@shaedrich
Copy link

The suggestions here are getting quite wild. This started out as a thin wrapper around parse_url and parse_str but people are adding and adding their wishes and ideas.

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.

@tontonsb
Copy link
Contributor

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 parse_url and parse_str with a minimal API or is it a complete URL handling tool with setters/withers and something better than parse_str underneath.

@taylorotwell
Copy link
Member

taylorotwell commented Nov 11, 2024

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 taylorotwell marked this pull request as draft November 11, 2024 21:22
@stevebauman
Copy link
Contributor Author

@taylorotwell Ok great, will do!

@stevebauman stevebauman marked this pull request as ready for review November 11, 2024 23:49
@stevebauman
Copy link
Contributor Author

stevebauman commented Nov 11, 2024

Ok all set! Most notably, I've updated the $query property of the Url class to be an instance of UrlQueryParameters.

Developers can change the object (and any of the properties) which will automatically be reflected when the Url is transformed into a string or array:

$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! 👍

@tontonsb
Copy link
Contributor

@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.

@shaedrich
Copy link

shaedrich commented Nov 12, 2024

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 parse_url and parse_str with a minimal API or is it a complete URL handling tool with setters/withers and something better than parse_str underneath.

@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.

grafik

Using these packages does sound interesting for subsequent versions, especially, since we won't have to maintain that much code on our own then.

@stevebauman
Copy link
Contributor Author

@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?

@timacdonald
Copy link
Member

I don't have strong opinions there. As I mentioned in my initial comment...

I always imagined the Laravel URL class would offer similar functionality to league\uri, with a more Laravel-style API.

Can just s/league/guzzle/ there.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.