[wp-trac] [WordPress Trac] #36292: Rewrites: Next Generation
WordPress Trac
noreply at wordpress.org
Fri Nov 13 20:24:09 UTC 2020
#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 fav05):
Hi, [https://www.bairesdev.com/blog/write-better-code/] an article
explaining what to consider when writing a better code.
Replying to [comment:12 giuseppe.mazzapica]:
> Replying to [comment:11 MikeSchinkel]:
> > Replying to [comment:10 giuseppe.mazzapica]:
> > > Replying to [comment:9 MikeSchinkel]:
> > > > I think both of the solutions suggested here might be missing
something.
> > > >
> > > > If hooks are used as @giuseppe.mazzapica then unit testing will be
more difficult if not close to impossible, and that is Part 2 of @rmccue's
requirements ''(which I definitely agree with.)''
> > >
> > > Why? Can you better explain this? If there will be a callback or an
object that receives an array of rules and do the parsing, you can easily
unit test that callback or object by passing to it an arbitrary set of
rules.
> >
> > Actually, you can test but you cannot really '''''unit''''' test.
> >
> > You ''can'' do is ''"integration"'' testing though, and that's what
your test suites in Cortex are doing.
> >
> > [https://www.toptal.com/qa/how-to-write-testable-code-and-why-it-
matters Here is an article] that explains the difference.
>
> Believe me, I know exactly what unit tests are. I wrote a library (http
://brain-wp.github.io/BrainMonkey/) for the very sole purpose of
'''unit''' testing WordPress plugins.
>
> You'll be surprised that my plugins are the very few open source
WordPress plugins on the entire web that are tested '''without''' loading
WordPress. Every WordPress class and function and hook is mocked.
>
> No, they are '''not''' integration tests.
>
> Moreover, you might find interesting ''my'' answer on WPSE
http://wordpress.stackexchange.com/questions/164121/testing-hooks-
callback/164138#164138 where I talk just about unit testing & WordPress.
>
> > Of course you can't really unit test ''anything'' that has a hook in
it, which indicts the majority of the WordPress extension API, and I am
fully aware of that.
>
> Using my Brain Monkey, I actually can.
>
> > As best I can tell from what you proposed the callback is not called
until ''after'' the regex matches, so unless I misunderstand your
suggestion the matches would still be context free.
> >
> > Maybe your definition of ''"context"'' is different than I intended
when I used it? The context I meant was that state of the system
configuration and/or data currently in the system. For example, if we
want Categories in the URL root and we have a `sports` category slug then
that URL would route to the Category Archive instead of routing to a Page
with a URL slug of `sports`. ''(And with the approach I proposed we'd
need to make sure users were warned when path segments became ambigous.)''
>
> In my first comment here
(https://core.trac.wordpress.org/ticket/36292?replyto=11#comment:4) I
proposed to provide context when adding hooks (via the introduction of a
new hook `add_rewrite_rules` that pass an instance of request object)
'''and''' to pass context route callbacks.
>
> Reason is that storing rules in DB, like WP does now, conditional rule
addition is not possible at all. If the flushing system will be refactored
(as I hope) a cache layer should probably implemented, and conditional
addition will break cache.
>
> So, when a route is ''slightly'' different based on context, is probably
convenient just ad it and then handle differences while performing route
action, which is
> easy and straightforward if route action is a callback that receives
context as argument.
>
> When differences are bigger, you can use the context to conditional
adding rules, assuming that flushing system is refactored.
>
>
> > P.S. BTW, I set up a test with Cortex and one thing I noticed was
quite a lot of overhead in the autoloading and the creation of all the
objects prior to getting to the point of calling `Router->match()`.
Further `Router->parseRoutes` does a lot of work on each page load that
could be cached. That includes `RouteParser->parse()` doing a
`preg_split()` and `RouteParser->parsePlaceholders() `doing a
`preg_match_all()` for each route for each page load.
>
> Yes, Cortex approach is '''not''' optimized. A system based purely on
FastRoute would be much (much) faster.
>
> The counter-optimization of Cortex has different reasons. Among others,
operating outside of WordPress core and wanting to have 100% core
compatibility, I have to include some additional overhead.
>
> Moreover, FastRoute have no system to differentiate routes based on
domain and have no way do define "priority" for routes nor to define route
"groups".
>
> FastRoute is committed to speed, so it reduces features to avoid
overhead, but I decided that some overhead is an acceptable trade off to
have those features.
>
> Even because I did some very basic profiles comparing Cortex with
WordPress native implementation and noted that, in real world use cases,
there is close to zero performance loss and even in worst case scenario
the performance loss is so trivial that is negligible compared to the
whole WordPress environment bootstrap process.
>
> (In the "Clever Rules" implementation wehre I used the URL chunking
approach performance loss was much bigger).
>
> So, by now, the flexibility and general benefits I get from Cortex
exceed by far the small performance issue.
>
> Finally, consider that Cortex is not stable yet, and one of the reasons
is that there is no cache layer yet and so route parsing / matching
happens on every request which, of course, is suboptimal.
>
> However, I use it in production in quite big sites (sorry, I can't share
it publicly, but if you are intersted I could privately) and the
performance impact is unnoticeable, lost in the much bigger "trouble" of
WordPress bootstrap.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/36292#comment:17>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list