[buddypress-trac] [BuddyPress Trac] #5436: BP_Core shouldn't run on 'bp_setup_components'

[buddypress-trac]

#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,
 etc...

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

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/5436#comment:13>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac