Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:124061
Feedback-ID: i7dd1477f:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-538-g1508afaa2-fm-20240616.001-g1508afaa
Precedence: bulk
MIME-Version: 1.0
Message-ID: <fa6fa2df-ec42-4dc4-9a1f-837383232bfc@app.fastmail.com>
In-Reply-To: 
 <CAH5C8xUb1O20ZDrOQNC=ckFxHUUWSK7sw_njQQzFBd0qgQqoww@mail.gmail.com>
References: 
 <CAH5C8xUb1O20ZDrOQNC=ckFxHUUWSK7sw_njQQzFBd0qgQqoww@mail.gmail.com>
Date: Sat, 29 Jun 2024 21:27:50 +0100
To: internals@lists.php.net
Subject: Re: [PHP-DEV] [RFC] [Discussion] Add WHATWG compliant URL parsing API
Content-Type: multipart/alternative;
 boundary=714affaf3b254a81bb42ddc2365d501e
From: krinkle@fastmail.com (Krinkle)

--714affaf3b254a81bb42ddc2365d501e
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Fri, 28 Jun 2024, at 21:06, M=C3=A1t=C3=A9 Kocsis wrote:
> [=E2=80=A6] add a WHATWG compliant URL parser functionality to the sta=
ndard library. The API itself is not final by any means, the RFC only re=
presents how I imagined it first.
>=20
> You can find the RFC at the following link: https://wiki.php.net/rfc/u=
rl_parsing_api

First-pass comments/thoughts.

As others have mentioned, it seems the class would/could not actually sa=
tisfy PSR-7. Realistically, the PSR-7 interface package or someone else =
would need to create a new class that combines the two, potentially as p=
art of a transition away from it to the built-in class, with future PSRs=
 building directly on Url. If we take that as given, we might as well de=
sign for the end state, and accept that there will be a (minimal) transi=
tion. This end state would benefit from being designed with the logical =
constraints of PSR-7 (so that migration is possible without major surpri=
ses), but without restricting us to its exact API shape, since an interm=
ediary class would come into existence either way.

For example, Url could be a value class with merely 8 public properties.=
 Possibly with a UrlImmutable subclass, akin to DateTime, where the prop=
erties are read-only instead a clone method could return Url?).

It might be more ergonomic to leave the parser as implementation detail,=
 allowing the API to be accessed from a single import rather than requir=
ing two. This could look like Url::parse() or Url::parseFromString().=20

For the Url::parseComponent() method, did you consider accepting the exi=
sting PHP_URL_* constants? They appear to fit exactly, in naming, descri=
ption, and associated return types.

Without UrlParser/UrlComponent, I'd adopt it direclty in applications an=
d frameworks. WIthout it, further wrapping seems likely for improved usa=
bility. This is sometimes benefitial when exposing low-level APIs, but i=
t seems like this is close to fitting in a single class, as demonstrated=
 by the WHATWG URL API.

One thing I feel is missing, is a method to parse a (partial) URL relati=
ve to another. E.g. to expand or translate paths between two URLs. Consi=
der expanding "/w/index.php", or "index.php" relative to "https://wikipe=
dia.org/w/". Or expanding "//example.org" relative to either "https://wi=
kipedia.org" vs "http://wikipedia.org". The WHATWG URL API does this in =
the form of a second optional string|Stringable parameter to Url::parse(=
). Implementing "expand URL"  with parsing of incomplete URLs is error-p=
rone and hard to get right.  Including this would be valuable.

See also Net_URL2 and its resolve() method https://pear.php.net/package/=
Net_URL2 https://github.com/pear/Net_URL2=20

--
Timo Tijhof
https://timotijhof.net/

--714affaf3b254a81bb42ddc2365d501e
Content-Type: text/html;charset=utf-8
Content-Transfer-Encoding: quoted-printable

<!DOCTYPE html><html><head><title></title><style type=3D"text/css">p.Mso=
Normal,p.MsoNoSpacing{margin:0}</style></head><body><div>On Fri, 28 Jun =
2024, at 21:06, M=C3=A1t=C3=A9 Kocsis wrote:<br></div><blockquote type=3D=
"cite" id=3D"qt" style=3D""><div dir=3D"ltr"><div>[=E2=80=A6] add a WHAT=
WG compliant URL parser functionality to the standard library. The API i=
tself is not final by any means, the RFC only represents how I imagined =
it first.<br></div><div><br></div><div>You can find the RFC at the follo=
wing link:&nbsp;<a href=3D"https://wiki.php.net/rfc/url_parsing_api">htt=
ps://wiki.php.net/rfc/url_parsing_api</a><br></div></div></blockquote><d=
iv><br></div><div>First-pass comments/thoughts.<br></div><div><br></div>=
<div>As others have mentioned, it seems the class would/could not actual=
ly satisfy PSR-7. Realistically, the PSR-7 interface package or someone =
else would need to create a new class that combines the two, potentially=
 as part of a transition away from it to the built-in class, with future=
 PSRs building directly on Url. If we take that as given, we might as we=
ll design for the end state, and accept that there will be a (minimal) t=
ransition. This end state would benefit from being designed with the log=
ical constraints of PSR-7 (so that migration is possible without major s=
urprises), but without restricting us to its exact API shape, since an i=
ntermediary class would come into existence either way.<br></div><div><b=
r></div><div>For example, Url could be a value class with merely 8 publi=
c properties. Possibly with a UrlImmutable subclass, akin to DateTime, w=
here the properties are read-only instead a clone method could return Ur=
l?).<br></div><div><br></div><div>It might be more ergonomic to leave th=
e parser as implementation detail, allowing the API to be accessed from =
a single import rather than requiring two. This could look like Url::par=
se() or Url::parseFromString().&nbsp;<br></div><div><br></div><div>For t=
he Url::parseComponent() method, did you consider accepting the existing=
 PHP_URL_* constants? They appear to fit exactly, in naming, description=
, and associated return types.<br></div><div><br></div><div>Without UrlP=
arser/UrlComponent, I'd adopt it direclty in applications and frameworks=
. WIthout it, further wrapping seems likely for improved usability. This=
 is sometimes benefitial when exposing low-level APIs, but it seems like=
 this is close to fitting in a single class, as demonstrated by the WHAT=
WG URL API.<br></div><div><br></div><div>One thing I feel is missing, is=
 a method to parse a (partial) URL relative to another. E.g. to expand o=
r translate paths between two URLs. Consider expanding&nbsp;"/w/index.ph=
p", or&nbsp;"index.php" relative to "<a href=3D"https://wikipedia.org/w/=
">https://wikipedia.org/w/</a>". Or expanding "//example.org" relative t=
o either&nbsp;"<a href=3D"https://wikipedia.org">https://wikipedia.org</=
a>" vs&nbsp;"<a href=3D"http://wikipedia.org">http://wikipedia.org</a>".=
 The WHATWG URL API does this in the form of a second optional string|St=
ringable parameter to Url::parse(). Implementing "expand URL"&nbsp; with=
 parsing of incomplete URLs is error-prone and hard to get right.&nbsp; =
Including this would be valuable.<br></div><div><br></div><div>See also =
Net_URL2 and its resolve() method <a href=3D"https://pear.php.net/packag=
e/Net_URL2">https://pear.php.net/package/Net_URL2</a>&nbsp;<a href=3D"ht=
tps://github.com/pear/Net_URL2">https://github.com/pear/Net_URL2</a> <br=
></div><div><br></div><div>--<br></div><div>Timo Tijhof<br></div><div><a=
 href=3D"https://timotijhof.net/">https://timotijhof.net/</a><br></div><=
div><br></div></body></html>
--714affaf3b254a81bb42ddc2365d501e--