Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124072
Precedence: bulk
MIME-Version: 1.0
References: <CAH5C8xUb1O20ZDrOQNC=ckFxHUUWSK7sw_njQQzFBd0qgQqoww@mail.gmail.com>
 <71a73b87-cc2f-4ee5-a961-7bf2b191fbb6@gmail.com>
In-Reply-To: <71a73b87-cc2f-4ee5-a961-7bf2b191fbb6@gmail.com>
Date: Sun, 30 Jun 2024 08:00:00 +0200
Message-ID: <CAH5C8xV-tLuNWmEiB8UTgpUbstfKVNBshQ6DFH2nQOHQPn=ATg@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add WHATWG compliant URL parsing API
To: Niels Dossche <dossche.niels@gmail.com>
Cc: internals@lists.php.net
Content-Type: multipart/alternative; boundary="00000000000068693b061c1537a1"
From: kocsismate90@gmail.com (=?UTF-8?B?TcOhdMOpIEtvY3Npcw==?=)

--00000000000068693b061c1537a1
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Niels,

First of all, thank you for your support!

Why did you choose UrlParser to be a "static" class? Right now it's just a
> fancy namespace.


That's a good question, let me explain the reason: one of my major design
goals was to make the UrlParser class to be
extendable and configurable (e.g. via an "engine" property similar to what
Random/Randomizer has). Of course, UrlParser
doesn't support any of this yet, but at least the possibility is there for
followup RFCs due to the class being final.

Since I knew it would be an overkill to require instantiating an UrlParser
instance for a task which is stateless (URL parsing),
finally I settled on using static methods for the purpose. Later, if the
need arises, the static methods could be converted to
non-static ones with minimal BC impact.

It's a bit of a shame that the PSR interface treats queries as strings.

In Javascript we have the URLSearchParams class that we can use as a
> key-value storage for query parameters.


Hm, yes, that's an observation I can agree with. However, this restriction
shouldn't limit followups to add key-value storage
support for query parameters. Although, as far as I could determine,
neither Lexbor is capable of such a thing currently.

Why is UrlComponent a backed enum?


To be honest, it has no specific reason apart from that's what I am used
to. I'm fine with whatever choice, even with getting rid of
UrlComponent completely. I added the UrlParser::parseUrlComponent() method
(and hence the UrlComponent enum) to the
proposal in order to have a direct replacement for parse_url() when it's
called with the $component parameter set, but I wasn't
really sure whether this is needed at all... So I'm eager to hear any
recommendations regarding this problem.

A nit: We didn't bundle the entire Lexbor engine, only select parts of it.
> Just thought I'd make it clear.


Yes, my wording was slightly misleading. I'll clarify this in the RFC.

About edge cases: e.g. what happens if I call the Url constructor and leave
> every string field empty?


Nothing :) The Url class in its current form can store invalid URLs. I know
that URLs are generally modeled as value objects (that's
also why the proposed class is immutable), and generally speaking, value
objects should protect their invariants. However, due to
separating the parser to its own class, I abandoned this "rule". So this is
one more downside of the current API.

Regards,
M=C3=A1t=C3=A9

--00000000000068693b061c1537a1
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">Hi Niels,<div><br></div><div>First of all=
, thank you for your support!</div><div><br></div><blockquote class=3D"gmai=
l_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,20=
4,204);padding-left:1ex">Why did you choose UrlParser to be a &quot;static&=
quot; class? Right now it&#39;s just a fancy namespace.</blockquote><div><b=
r></div><div>That&#39;s a good question,=C2=A0let me explain the reason: on=
e of my major design goals was to make the UrlParser class to be</div><div>=
extendable and configurable (e.g. via an &quot;engine&quot; property simila=
r to what Random/Randomizer has). Of course, UrlParser</div><div>doesn&#39;=
t support any of this yet, but at least the possibility is there for follow=
up RFCs due to the class being final.</div><div><br></div><div>Since I knew=
 it would be an overkill to require instantiating an UrlParser instance for=
 a task which is stateless (URL parsing),</div><div>finally I settled on us=
ing static methods for the purpose. Later, if the need=C2=A0arises, the sta=
tic methods could be converted to</div><div>non-static ones with minimal BC=
 impact.</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"mar=
gin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1=
ex">It&#39;s a bit of a shame that the PSR interface treats queries as stri=
ngs.</blockquote><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px =
0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">In Javas=
cript we have the URLSearchParams class that we can use as a key-value stor=
age for query parameters.=C2=A0</blockquote><div><br></div><div>Hm, yes, th=
at&#39;s an observation I can agree with. However, this restriction shouldn=
&#39;t limit followups to add key-value storage</div><div>support for query=
 parameters. Although, as far as I could determine, neither Lexbor is capab=
le of such a thing currently.</div><div><br></div><blockquote class=3D"gmai=
l_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,20=
4,204);padding-left:1ex">Why is UrlComponent a backed enum?</blockquote><di=
v><br></div><div>To be honest, it has no specific reason apart from that=
9;s what I am used to. I&#39;m fine with whatever choice, even with getting=
 rid of</div><div>UrlComponent completely. I added the UrlParser::parseUrlC=
omponent() method (and hence the UrlComponent enum) to the</div><div>propos=
al in order to have a direct replacement for parse_url() when it&#39;s call=
ed with the $component parameter set, but I wasn&#39;t</div><div>really sur=
e whether this is needed at all... So I&#39;m eager to hear any recommendat=
ions regarding this problem.</div><div><br></div><blockquote class=3D"gmail=
_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204=
,204);padding-left:1ex">A nit: We didn&#39;t bundle the entire Lexbor engin=
e, only select parts of it. Just thought I&#39;d make it clear.</blockquote=
><div><br></div><div>Yes, my wording was slightly misleading. I&#39;ll clar=
ify this in the RFC.<br></div><div><br></div><blockquote class=3D"gmail_quo=
te" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204=
);padding-left:1ex">About edge cases: e.g. what happens if I call the Url c=
onstructor and leave every string field empty?</blockquote><div><br></div><=
div>Nothing :) The Url class in its current form can=C2=A0store invalid URL=
s. I know that URLs are generally modeled as value objects (that&#39;s</div=
><div>also why the proposed class is immutable), and generally speaking, va=
lue objects should protect their=C2=A0invariants. However, due to</div><div=
>separating the parser to its own class, I abandoned this &quot;rule&quot;.=
 So this is one more downside of the current API.</div><div><br></div><div>=
Regards,<br>M=C3=A1t=C3=A9</div></div></div>

--00000000000068693b061c1537a1--