[buddypress-trac] [BuddyPress Trac] #7218: Only load component action and screen code when we're on the component's page
buddypress-trac
noreply at wordpress.org
Tue Feb 27 20:31:29 UTC 2018
#7218: Only load component action and screen code when we're on the component's
page
-------------------------+-----------------------
Reporter: r-a-y | Owner:
Type: enhancement | Status: reopened
Priority: normal | Milestone: 3.0
Component: Core | Version:
Severity: normal | Resolution:
Keywords: has-patch |
-------------------------+-----------------------
Comment (by boonebgorges):
Thanks @r-a-y :-D
The screen/action OCD patch seems fine to me. Any performance benefits to
splitting the files up will be extremely minimal, but the main benefit -
of better organized routing functions - seems worth the change. If there
are no other objections from the peanut gallery, let's go with this
pattern.
> I've chosen to run this on the 'bp_setup_canonical_stack' hook at
priority 20
Got it. I'd chosen `'bp_template_redirect'` because that's closer to when
the `'bp_actions'` and `'bp_screens'` hooks are actually fired, but you're
right that this makes it very hard to unhook them. The only suggestion I'd
make is that piggybacking on `bp_setup_canonical_stack` feels brittle to
me, especially since the canonical-stack logic isn't 100% connected to the
determination of the current component. Since `bp_setup_canonical_stack`
is run at `bp_init::5`, perhaps `bp_late_includes()` could be hooked at
`bp_init::7` or something like that. This would be much easier to reason
about in `bp-core-actions.php`, and is still early enough for
`bp_init::10` callbacks, which I think is our main concern.
> I'm not a fan of the existing BP_Component::includes() method as it does
too many checks for superfluous files
Yup, totally agreed, I was just throwing something together :-D
--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7218#comment:59>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac
More information about the buddypress-trac
mailing list