ClassicPress / classic-commerce

A simple but powerful e-commerce platform built for ClassicPress. Forked from WooCommerce and compatible with many Woo extensions.
https://classiccommerce.cc/
GNU General Public License v3.0
54 stars 15 forks source link

Calling wp_die() in the AJAX handlers of WC_AJAX makes it difficult to reuse code. #320

Closed magenta-cuda closed 3 years ago

magenta-cuda commented 3 years ago

https://github.com/ClassicPress-plugins/classic-commerce/blob/10650f8347d4307b8b02ccdc76195e544cb50e51/includes/class-wc-ajax.php#L608-L656

I think it in general it is a bad idea to call wp_die() in the main code of an AJAX handler. I will use the WC_AJAX::save_attributes() AJAX handler as an example. But, in fact many of the AJAX handlers of WC_AJAX call wp_die().

The problem with calling wp_die() or functions that call wp_die() e.g., wp_send_json_success(), wp_send_json(), ... is that you cannot easily reuse code by wrapping the AJAX handler. I want to do the following:

remove_action( 'wp_ajax_woocommerce_' . 'save_attributes', [ 'WC_AJAX', 'save_attributes' ] );
add_action(    'wp_ajax_woocommerce_' . 'save_attributes', 'my_save_attributes' );

function my_save_attributes() {
    # pre process stuff
    WC_AJAX::save_attributes();
    # post process stuff
}

However, this will not work because WC_AJAX::save_attributes() calls wp_send_json_success() which calls wp_send_json() which calls wp_die() and hence WC_AJAX::save_attributes() never returns and the post process stuff is never done.

If the main code of save_attributes() was in a separate method

function save_attributes() {
    $response = WC_AJAX::save_attributes_main();
    if ( array_key_exists( 'error', $response ) ) {
        wp_send_json_error( $response );
    } else {
        wp_send_json_success( $response );
    }
}

public static function save_attributes_main() {
    ...
    try {
        ...
        # wp_send_json_success( $response );
        return $response;
    } catch ( Exception $e ) {
        # wp_send_json_error( array( 'error' => $e->getMessage() ) );
        return array( 'error' => $e->getMessage() );
    }
}

Then I can call WC_AJAX::save_attributes_main() from my_save_attributes:

function my_save_attributes() {
    # pre process stuff
    $response = WC_AJAX::save_attributes_main();
    if ( array_key_exists( 'error', $response ) ) {
        # post process stuff
        wp_send_json_error( $response );
    } else {
        # post process stuff
        wp_send_json_success( $response );
    }
}

Currently, as a workaround I am using the filter 'wp_die_ajax_handler' but this is really not ideal as the resulting code is confusing.