[wp-trac] [WordPress Trac] #36292: Rewrites: Next Generation

[WordPress Trac]

#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