[buddypress-trac] [BuddyPress Trac] #5436: BP_Core shouldn't run on 'bp_setup_components'
noreply at wordpress.org
Sat Jul 5 18:15:06 UTC 2014
#5436: BP_Core shouldn't run on 'bp_setup_components'
Reporter: r-a-y | Owner:
Type: defect (bug) | Status: new
Priority: high | Milestone: 2.1
Component: Core | Version:
Severity: normal | Resolution:
Keywords: has-patch 2nd-opinion |
Comment (by boonebgorges):
Thanks for chiming in, johnjamesjacoby, and thanks for the sanity check :)
> I'm opposed to doing too much rearranging of actions or priorities until
we can clearly identify the nuances that are causing issues.
I think that a fair amount of identification of nuances has happened in
the discussion and patches above. In a nutshell: as BP has grown, we've
lumped together the "component setup process" into chunks that are too
large, and depending on accidental features of the WP plugin API (such as
the fact that multiple callbacks attached to a single hook with the same
priority of 10 will be fired in the order in which they were added) to
paint over the fact that we're being too coarse-grained. This manifests
itself in a couple different ways, but the overarching theme is that we're
trying to do too much work in each component's `setup_globals()`: we set
up basic info like slugs and database tables, then we determine stuff like
whether we're looking at a specific user or group, then we do access
control, then we do canonical URL setting, etc etc etc.
So, for example: if BP_Groups loads before BP_Members, it can't make
reference to anything about the BP_Members component - even basic stuff
like the members slug - when setting up its canonical URLs. This is a
basic design failure. Each component should register its global values;
*then* each component should determine necessary actions for the currently
requested page; *then* each component should set up its default
actions/canonical URLs; and so on. By breaking the setup routines into
smaller chunks and then serializing them across components, we greatly
decrease the potential for conflicts.
5436.04.patch is a very conservative attempt to address the worst parts of
this problem. It takes one important part of component setup - setting up
canonical URLs - and splits it into a separate BP_Component method that
fires *after* 'setup_globals'. This fixes the worst problems. It also
removes some of the hardcoded references to other components from
`setup_globals()` into standalone filters. So, it doesn't solve all
potential interdependence problems (and certainly doesn't break things up
into the small chunks that I think would be ideal) but it solves all of
the interdependency issues that actually plague BP at the current moment.
I think that's enough for a first pass.
So, in the absence of argmuent to the contrary, I'm going to suggest that
we go ahead with 5436.04.patch for 2.1, and we ramp up our vigilance
regarding atomic methods in the future :)
> Not saying this is the way we should go, but bbPress separates "common"
code from "core" code (the same way we separate Member code from Core.)
> For predictability of use, the Core component should be subjected to all
the same actions other components are (I'd marry it to the bp_include
action if we could) so that it can easily be overridden, hooked into,
These distinctions are helpful, though I draw a different conclusion from
them. In my view, what we call "BP_Core" is not really a "component" at
all. It doesn't really set up navigation, or require its own tables, or do
pretty much anything that the other components do. Nearly all of it counts
as "common" in my view. And as such, it should *not* be subjected to the
same actions as other components (and in fact, maybe it shouldn't use
BP_Component at all). Probably a discussion for another day, and
definitely needs more reseach (BP_Core is huge), but that's my initial
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5436#comment:13>
BuddyPress Trac <http://buddypress.org/>
More information about the buddypress-trac