Hey Tim,
Thank you for the help, I forgot to link the RFC indeed.
Let me answer your questions:
>
> I don't think it is as necessary to make it readonly as it is with the
> URI classes, since the query parameters class is likely to be used for
> “local modification” and is less likely to be passed around as a value
> object. If efficient copy-on-write is possible internally, then making
> it readonly is the safer choice, of course.
>
Yes, as far as I know from Ilija, it's possible to optimize modification
based on
refcount: if a readonly property which is to be modified has a refcount of
1 and
there's no weakref, it can be safely updated in-place.
> With regard to the RFC text, I've given it a first quick skim and have
> the following notes:
>
> 1. Within the "Relation to the query component" section.
>
> The `$uri = new Uri\Rfc3986\Uri("https://example.com?foo=a b");` example
> is invalid. An URI containing a space is not valid and the `Uri` class
> will throw an exception. Can you double-check that part of the RFC?
> Without looking it up, I suspect that the space should be `%20` for
> RFC3986?
>
Nice catch, I'll review and fix the examples using "a b". It is indeed
incorrect, and %20
should be used instead. as you said.
> 4. Within the "Supported types" section.
>
> “The above conversion rules work for both UriQueryParams and
> UrlQueryParams.”
>
> and the examples also use Uri\Rfc3986\UriQueryParams /
> Uri\WhatWg\UrlQueryParams.
>
> This seems like an oversight from the original implementation, since
> there are no longer separate classes. Can you double-check the entire
> section?
>
I intentionally haven't updated this section yet: because there's important
info about
the difference between how WHATWG specification and uriparser handles null,
so I didn't want to "lose" this (I know it remains in the changelog).
>
> 5. Within the "Array API" section.
>
> This also uses Uri\Rfc3986\UriQueryParams in the examples
Same situation as above, but basically the whole section is a big question
mark:
I couldn't come up with a good enough idea how arrays could be handled
efficiently and ergonomically, and in the same time, how to (mostly) conform
to WHATWG URL.
Some of my problems:
- How should $params->get("foo[bar]") behave? Should we try to parse the
supplied key as an array and return the
nested item if it exists? I suppose, we would internally store the query
params as an array (just like how $_GET does),
so then we would definitely need parsing in order to be able to return item
"bar" stored within array "foo".
- However, what should a $params->append("foo[bar]") call do when the query
param "foo" is already added with a string value?
Should it modify the existing item to be an array (but then how?) or we
should just add "foo[bar]" to the query params as-is?
- How to recompose list parameters? Ideally, we should not append [] to
these parameters (e.g.
"list=1&list=2" vs "list[]=1&list[]=2"), and actually, it is not needed for
arrays in the root level, but as far as I understood,
it's a must-have for nested lists (e.g. "map[list]=1&map[list]=2" vs.
map[list[]]=1&map[list[]]=2), otherwise the representation
would be ambiguous for lists with 1 item: "map[list]=1" would mean that the
map has a list item with a value of "1". So
we either always have to use the [] suffix for lists, or we should make a
distinction between root-level and nested lists.
I know, most of my questions revolve around parsing the $name parameter,
but I would really want to minimize the number
of times when the $name param has to be parsed run-time so that the
performance is comparable to native array key
lookups. I don't have any idea how much the parsing would cost though...
So at this point, coming up with a sensible plan is needed, and I can fix
most inconsistencies in the RFC afterwards.
Regards,
Máté
