[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 giuseppe.mazzapica):

 Replying to [comment:5 rmccue]:

 > 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.

 Per my understanding current format is

 {{{
 add_rewrite_rule(
   "^{$custom_date_format}-{$timespan_format}/?",
   'index.php?somevar=somevalue',
   'top'
 );
 }}}


 It means you have to distinguish `index.php?somevar=somevalue` from a
 callback. There's no possible overlap, especially if you check for
 `is_callable`, because the only way a string can be callable is when it
 contains a function name, but a function name can't contain `=` which is
 required in the old format.

 > 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. :)

 Sure, rework flushing is a priority. This is not the only issue with
 current flushing system. I perfectly agree that a cache for rules is
 propably needed, but cache !== storage.

 > 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
 callback-rule.php​ for how this could work.

 Converting public API to internal object, by the way, is a thing that
 pretty much all the libraries out there (including FastRoute) are doing,
 and I don't see any real problem in it. By the way, your callback rule
 class is not that bad.

 > 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.

 Yes, but `[] !== ''`. In other words, if an array is returned (even empty)
 you can assume this is a query argument array. If anything that is not an
 array is returned (no matter what it is) you can assume you want to skip
 main query. This should be pretty straightforward to grasp for users.

 > 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.

 Well, the callback is not executed at parsing stage. It is executed when
 the parsing stage ended. At that point, there are few things that happen.
 Some of them are query related and the implementation you added to OP
 already skip them when you choose to skip the main query.

 The most important thing that happen before `wp` in the scenario we are
 skipping main query is the HTTP header settings. However, if we want to
 have the maximum flexibility, I think is fine if the rule callback has the
 possibility to complete control the response, which includes HTTP headers.

 In short, I think that if the target is to decouple url from query (which
 is something I really agree) `wp` is probably too late.

 Regarding the hook `wp` itself, IMO it is a query-related hook as well.
 Anything existing out there (plugins, themes) that is using the hook was
 coded with the current WP implementation in mind, where each frontend
 request trigger a query.

 If this new approach lands in core, a callback that takes complete control
 over the request has nothing to gain from firing `wp`, because anything
 that hooks there is expecting a query, which may not happen.

 On the other hand, if the callback returns an array, the query will happen
 and `wp` will be fired as usual, and everyone will be happy.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/36292#comment:6>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform