[wp-trac] [WordPress Trac] #36292: Rewrites: Next Generation
WordPress Trac
noreply at wordpress.org
Wed Mar 23 02:20:17 UTC 2016
#36292: Rewrites: Next Generation
-----------------------------+-----------------------------
Reporter: rmccue | Owner: rmccue
Type: feature request | Status: assigned
Priority: normal | Milestone: Future Release
Component: Rewrite Rules | Version:
Severity: normal | Resolution:
Keywords: | Focuses:
-----------------------------+-----------------------------
Comment (by rmccue):
Replying to [comment:4 giuseppe.mazzapica]:
> I really like the effort on this.
Thanks for posting your thoughts! Replies inline.
> A class for each a rule is overwhelming imo.
>
> I think that would be easier and with less impact, to introduce a new
function, maybe named something like `add_rewrite_callback`.
On the surface, I agree, I'd prefer a straight-up callback too. However,
there's a few concerns with that, which is why I made this a
class/interface:
* The current format is a string/array. Callbacks in PHP are
strings/arrays (or Closure objects), which means the domain overlaps. This
potentially leads to conflicts and a backwards compatibility break, as we
can't easily tell the two types apart (i.e. if I have the string set to
`reset`, do I want to call that function, or set an empty query var? It
gets worse when you consider array query vars). We _could_ break this, but
it's not necessary.
* We have more than one callback here, and I'd actually like to add a few
more. The design of this is based partially around hm-rewrite, which
avoids needing conditionals later on in hooks.
* If we have multiple bits of data, the traditional way in WP-land is to
use arrays for that. However, because this is part of the critical path
(i.e. must be run on every request), performance is a concern. Arrays
actually eat up more memory quite quickly (per
[https://gist.github.com/nikic/5015323 nikic], "`104 + 96*n` for arrays
and `128 + 8*n` for objects"), so objects are the more lightweight
approach here, especially for newer PHP versions.
(See below for potential solution.)
> As you can see, I passed the yet to come `WP_HTTP_Request` to a new hook
`'add_rewrite_rules'` that, passing the HTTP request, easily allows to add
rules only under some request conditions, for instance, HTTP method.
Until we rework how flushing works, conditional registration won't work. I
want to do this step-by-step so we can assess as we change, but this may
be possible some day. :)
I do like the approach in your next piece of code of passing the Request
object in to the callback though. This would allow us to implement per-
method rules too through some slight reworking of the current code
(replacing `get_verbose_page_match` with a `is_valid`).
> This is quite an edge case, most of the rules won't require all this
code.
>
> Maybe, internally the rule callback can be used to generate instances of
a route class, but to require user to write a class for each custom rule
is far from ideal as a public API.
Doing it in the internal API might be a bit too polymorphic, but I would
like to simplify, so potentially a built-in wrapper could help? See
[attachment:callback-rule.php] for how this could work.
> Regarding skipping the main query, if the callback would return anything
that is not an array, then there are no variables to assign to the main
query, so you can just skip the main query if the callback return anything
but an array.
Not necessarily true; an empty query could simply be asking for the
defaults for WP_Query. `get_posts([])` e.g. is an empty query, but just
uses the defaults.
> Or maybe, would be possible to just `add_filter( 'skip_main_query',
'__return_true' )` inside the callback.
Potentially, but the aim here is to move away from the paradigm of rule ->
query, so I'd like the API to recognise the difference. Having to add a
filter is a bit of a pain :)
> Also note that this approach easily allows for things like:
...
> If you combine this with the possibility to add rules being aware of
specific conditions in the HTTP request (because `add_rewrite_rules` hook
pass the request object and the rule callback receives the request object
as argument), things becomes interesting.
Running a callback during the parsing stage is a bit too early, but I
definitely see the benefit of being able to just register a callback.
Ideally, we want this to run on the `wp` hook.
Added an example of how this can work currently in [attachment:just-
callback.php], but I want to explore this idea a bit more to make it a bit
easier.
> Consider that, since you can distinguish "old" string rules from this
new callback rules, you are not forced to convert the old ones, saving
time and memory.
That's possible currently, I mainly did the convert-all-of-them for
internal consistency, but it's not strictly required. (See above for
problems with differentiating between callbacks and queries though.) See
[comment:3] about that. :)
> As a final thought, my Cortex (https://github.com/Brain-WP/Cortex) uses
the library FastRoute (by Nikita Popov, one of the PHP devs who
contributed more to PHP 7) that is faster and less resources consuming of
the WP system because it use a very clever approach described in this blog
article http://nikic.github.io/2014/02/18/Fast-request-routing-using-
regular-expressions.html
>
> I think would be interesting explore the same concept for WordPress.
Absolutely agree, particularly in regards to FastRoute; my original draft
of the ticket included that, but I cut it out as it was already getting
long. :) Once we tackle part 2, we'll be able to investigate changing the
internals of the rewriting system much, much easier.
(If there's anything you disagree with here, please do let me know :D)
--
Ticket URL: <https://core.trac.wordpress.org/ticket/36292#comment:5>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list