Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 20 months ago

#7549 closed enhancement (maybelater)

Only load AJAX code on AJAX requests

Reported by: r-a-y Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Core Keywords: 2nd-opinion dev-feedback
Cc: brajesh@…, contato@…

Description

Similar to #7218, we should only load code hooked to AJAX hooks (those prefixed with wp_ajax_) on AJAX requests.

No need to run this code at all times.

Change History (12)

#1 @sbrajesh
2 years ago

It may be over optimization. Do you have any benchmark to support that idea that it significantly degrades the performance?
Loading extra code may affect memory usage but should not cause performance problem.

#2 @sbrajesh
2 years ago

  • Cc brajesh@… added

#3 @r-a-y
2 years ago

It doesn't significantly degrade performance, but why load unnecessary code when you don't need it?

We do not have to follow WordPress here :)

#4 @sbrajesh
2 years ago

Good point.
The problem is BuddyPress code base is more like a platform(Just like WordPress). Third party plugins build components with it without worrying about function/class check for every individual identifier. All they have to do is check BuddyPress is active and component is enabled.

Any hard coded optimization for loading functions/classes will have immense side effect. All the 3rd party code will need to check and load and repeat.

The solution that will be more useful is to use a proper autoloader(may be based on the PSR, modified for wp context) and write a wrapper for attaching actions. Decouple action handlers from the code attaching actions.

That way, Only wrappers will be required to load, the Action Handlers can be lazy loaded.

#5 @r-a-y
2 years ago

You should be most concerned about #7218 and not this ticket. #7218 will autoload the component action and screen code.

However, for this ticket, I only want to load AJAX when absolutely necessary since AJAX code has no usage outside of AJAX requests.

Something like this:

<?php
add_action( 'bp_include', function() {
        // AJAX condition.
        if ( defined( 'DOING_AJAX' ) && true === DOING_AJAX ) {
                // This file would contain any function hooked to 'wp_ajax_{WHATEVER}'
                require 'ajax.php';
        }
} );

Can you give me a use-case where this would be a problem? Would you call an AJAX function on the frontend? I'm not sure why anyone would do that.

If we're that concerned about breakage, we can go the autoload route.

Last edited 2 years ago by r-a-y (previous) (diff)

#6 @sbrajesh
2 years ago

Thank you.
You are right about the other ticket. I am waiting for the 3.0 dev cycle to test the things and add my feedback there.

For this ticket, if you are only checking if it is an ajax request(like above), then It won't break anything and it is the right way to do things. I had the concern that you were going to check for individual actions.

No need for an autoloader here but when 3.0 cycle starts, Will certainly be involved on the other tickets with that idea.

Keep up the good work @r-a-y

#7 @r-a-y
2 years ago

Thanks for talking this through, @sbrajesh. Looking forward to your feedback during the 3.0 cycle.

#8 @espellcaste
2 years ago

  • Cc contato@… added

#9 @r-a-y
21 months ago

  • Keywords 2nd-opinion dev-feedback added
  • Milestone changed from 3.0 to Under Consideration

This ticket would only benefit bp-legacy, but we're not really supporting bp-legacy anymore.

I would still like to patch bp-legacy to do this, but would need consensus from other devs before moving forward.

#10 @hnla
21 months ago

Might not be supporting however guessing plenty of installs will remain on legacy so an improvement like this would seem justified, think other enhancements less so and would tend to dilute the case to moving to something we hope is not an option so much as a new better default(Nouveau.

#11 @DJPaul
21 months ago

All efforts must go into making Nouveau be the best set of templates we possibly can; the best code quality, the best documentation, the best UI, the best Javascript, the best CSS. Our effort might not exceed the "best" that other people can produce, but given our pool of contributors, we always punch above our weight, which I've always been very impressed by, and thankful for.

As @hnla implied, making the Legacy templates better doesn't achieve success for Nouveau.

I am very reluctant to make this kind of optimisation to Legacy, given its age and - hopefully - the number of 3rd party themes and customisations relying on it, and the opportunity cost (vs. making Nouveau better).

#12 @DJPaul
20 months ago

  • Milestone Under Consideration deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Given no feedback in the last month, I'm going to close this. If a core team member wants to do this to BP-Legacy and can do so in a very backwards compatible fashion, please just do so.

Note: See TracTickets for help on using tickets.