On Sat, Dec 6, 2025, at 5:45 AM, Máté Kocsis wrote:
> Hi Larry,
>
>> - I really, really hate the "set" prefix on all the methods. It's a builder
>> object, surely the "set" is implied?
>
> I like that the "set" prefix makes all setters grouped together. For
> example, should we have to add some extra methods (like authority()),
> the build() method would appear between authority() and fragment() in
> IDE autocomplete lists without the set() prefix.
That seems like a very minor point. Code will be read hundreds of times more
than it is auto-completed...
>> - It really feels like there's an interface to extract here from the
>> Url/UriBuilder classes. There's literally only one type-specific method
>> (build()).
>
> It's the very same thing that we discussed for a very long time last
> time... What would be the purpose of the interface? To make the two
> builders
> interchangeable? But they produce fundamentally different URIs. Even if
> we don't include build(), then we still have some differences between
> the two implementations:
I guess what's bugging me here is that in the typical case, URLs and URIs are
interchangeable. If you're just using a standard ASCII domain and path, which
is the vast majority of URLs, then either one gets you the same result. So it
feels grating to have to deal with two different versions of that logic; say,
if I just want to pull the path off of one, or set the path on one.
I 1000% realize that's not your fault, nor PHP's fault, it's the fault of the
two competing standards that don't talk to each other. The theoretical Venn
diagram overlap of URLs and URIs is relatively small. The practical overlap is
quite large. So that makes ignoring the overlap very grating. Hence why I
keep looking for places to codify the safe overlap.
>> - UriQueryParams::hasWithValue(), could that be just hasValue()? You still
>> need to specify the key anyway, and that's self-evident from the signature.
>
> Yes, I was already considering updating this name, but I was sure that
> someone (with 99% confidence of you) will point this out and suggest a
> better one.
Nice to know I'm predictable? :-)
> I agree that hasValue() is probably the right choice, although
> hasNameAndValue() would be the most technically correct name...
>
>> - There's a count() method, so shouldn't Ur{i|l]QueryParams implement
>> Countable?
>
> Yes, it can. I thought that implementing Countable on its own (without
> IteratorAggregate) was less useful, so I omitted it. Ignace suggested
> another approach that
> would allow implementing IteratorAggregate: if it happens then I'm
> totally fine with also implementing Countable.
I don't feel strongly about IteratorAggregate. I'm not entirely sure I see the
value for that. But Countable seems like a no-brainer to include.
>> - Why both Uri getRawQueryParams() and getQueryParams()? It looks like they
>> would return the same value, no? (If not, that should be explained.)
>
> This is actually already briefly explained in the RFC (but thanks to
> Ignace how also described this part):
>
>> The difference between Uri\Rfc3986\Uri::getRawQueryParams() and
>> Uri\Rfc3986\Uri::getQueryParams() is that the former one passes the “raw”
>> (non-normalized) query string as an input when instantiating
>> Uri\Rfc3986\Uri\UriQueryParams.
I have read that sentence 3 times and it's still not making sense in my head.
Can you clarify with an example (in the RFC)?
>> - The sort() method... should it take an optional user callback, or do we
>> lock people in to lexical ordering?
>
> Only WHATWG URL specifies its behavior, and it uses basic alphanumeric
> sorting. Even though there's nothing that could stop us from
> implementing fancier
> sorting ways, I think it's already fine as-is. Sort() can be used to
> guarantee that the query components are in deterministic order, and I
> think that's all that we need.
Fair enough.
>> - It would be quite convenient of set() and append() returned $this,
>> allowing them to be chained.
>
> That's fine for me. WHATWG URL specifies their return type as void, so
> I went with this, but there's nothing wrong with returning $this.
It would certainly make for cleaner code. Possibly sort() should also return
$this.
My thinking is that half the time we'll just be inlining a builder call
somewhere and directly passing it to the appropriate UR object, so the more we
can avoid "ugh, I must have a temp variable here for some damned reason", the
better.
>> - The fromArray() logic is... totally weird and unexpected and I hate it.
>> :-) Why can't you support repeated query parameters using nested arrays
>> rather than gumming up all calls with a wonky format?
>
> Do you mean something like ["foo" => [0, 1, 2, 3]])"? I think it is
> indeed possible to implement what you suggested. Whenever the basic
> structure of the proposal settles a little bit,
> I'll update the implementation, and I'll try to find out a sensible
> behavior for arrays/objects.
Yes, that's more what I was thinking. Thanks.
>> - It's not clear how one would start a new query from scratch, with the
>> private constructor. There doesn't seem to be a justification for the
>> private. I can't see why new UriQueryParams()->set('foo', 'bar') is a bad
>> thing.
>
> Yes, starting from scratch is only possible by using
> UriQueryParams::fromArray([]) or UriQueryParams::parse(""). But I don't
> have any fundamental issue with
> adding support for the empty constructor variant.
An empty constructor would help make it more ergonomic, yes. Or, heck,
UriQueryParams::new() would also work, and avoid the edge cases of constructor
calls. :-) Just some more natural way to "start from scratch."
>> - Url::isSpecial() Could we come up with a better name here? "Special"
>> could mean anything unless you know the RFC; it feels like "real escape
>> string" all over again.
>
> The "special URL" is indeed the technicus terminus that WHATWG URL
> uses. The RFC explains the concept briefly:
>
>> The WHATWG URL specification defines some special schemes (http, https, ftp,
>> file, ws, wss), which have distinct parsing and serialization rules.
>
> I don't have any issues with the current name, but the only alternative
> I could imagine is Uri\WhatWg\Url::isSpecialScheme().
That would be imperfect, but still a major improvement from just `isSpecial()`,
as it specifies that it's the scheme that's special, not the whole URL. What
"special" means is still unclear, but at least the scope is reduced. IOW, yes
please.
--Larry Garfield
