WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#38889 closed enhancement (maybelater)

Proposition to allow an api.Value() using asynchronous callbacks

Reported by: nikeo Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: reporter-feedback
Focuses: javascript Cc:
PR Number:

Description (last modified by westonruter)

When an api.Value instance is set to a new value, we might need to wait for all callbacks to be completed or resolved before actually doing something else.
With the current implementation, if an api.Value has asynchronous callbacks, there's no way to know when it's resolved after a set() action. Any further dependant actions would have to be done after an hazardous delay.

The following is a proposition to solve this by adding an optional { deferred : true } param when binding a callback to an api.Value instance.

This way, a syntax like this could be used :

//let's declare a new api Value().
api.observable_value = new api Value( 'initial' );

//this is basic callback with a $.Deferred callback
var fakeAjaxCall = function( to, from ) {
    var dfd = $.Deferred();

    _.delay( function() {
        console.log( 'I am a fake ajax call.');
        dfd.resolve( to );
    }, 2000 );

    return dfd.promise();
};

//let's bind the Value instance with this callback and inform it that it is a deferred action
api.observable_value.bind( fakeAjaxCall, {deferred : true } );

//now let's set update the value
api.observable_value.set( 'new_val' ).done( function() {
    //do something with this new_val when all callbacks are done
    console.log('All callbacks are done now, we can use the new val in further dependant actions : ', this() );
});

This could be useful for callbacks involving preview refreshs ( if the preview returns a promise() , see #38797), or media fetched in ajax in db.

Code
This is a proposition of modification of the api.Value constructor, in the current customize-base.js file. ( as of 4.7 beta-3 )

The code introduces an optional additional collection of $.Deferred functions, as a property of the api.Value constructor.

/**
 * Set the value and trigger all bound callbacks.
 * @return a promise onto which it is attached the current 'this' of the api.Value, when all callbacks are resolved.
 * @param {object} to New value.
 */
set: function( to ) {
    var from = this._value, dfd = $.Deferred(), self = this, _promises = [];

    to = this._setter.apply( this, arguments );
    to = this.validate( to );

    // Bail if the sanitized value is null or unchanged.
    if ( null === to || _.isEqual( from, to ) ) {
      return this;
    }

    this._value = to;
    this._dirty = true;

    if ( this._deferreds ) {
          _.each( self._deferreds, function( _prom ) {
                _promises.push( _prom.apply( null, [ to, from ] ) );
          });

          $.when.apply( null, _promises )
                .then( function() {
                      self.callbacks.fireWith( self, [ to, from ] );
                      dfd.resolveWith( self, [ to, from ] );
                });
    } else {
          this.callbacks.fireWith( this, [ to, from ] );
          return dfd.resolveWith( self, [ to, from ] ).promise( self );
    }
    return dfd.promise( self );
}


/**
 * Bind a function to be invoked whenever the value changes.
 *
 * @param {...Function} A function, or multiple functions, to add to the callback stack.
 * @param { deferred : true } let us decide if the callback(s) have to be populated as $.Callbacks or $.Deferred
 */
bind: function() {
  var self = this,
      _isDeferred = false,
      _cbs = [];

  $.each( arguments, function( _key, _arg ) {
        if ( ! _isDeferred )
          _isDeferred = _.isObject( _arg  ) && _arg.deferred;
        if ( _.isFunction( _arg ) )
          _cbs.push( _arg );
  });

  if ( _isDeferred ) {
        self._deferreds = self._deferreds || [];
        _.each( _cbs, function( _cb ) {
              if ( ! _.contains( _cb, self._deferreds ) )
                self._deferreds.push( _cb );
        });
  } else {
        //original method
        self.callbacks.add.apply( self.callbacks, arguments );
  }
  return this;
}

The unbind method of the api.Value constructor is missing here, it should also be modified.
Any thoughts or feedbacks are welcome !

Change History (8)

#1 @nikeo
3 years ago

Typo : unpredictable would be more appropriate than hazardous

#2 @westonruter
3 years ago

  • Description modified (diff)
  • Keywords reporter-feedback added

@nikeo I'm not sure I understand. In your example here:

api.observable_value.bind( fakeAjaxCall, {deferred : true } );

api.observable_value.set( 'new_val' ).done( function() {
    //do something with this new_val when all callbacks are done
    console.log('All callbacks are done now, we can use the new val in further dependant actions : ', this() );
});

In this case, the value of “new_val” is basically temporary throw-away, isn't it? You're ultimately wanting to get the value from an Ajax request? It seems to me that you should rather just call fakeAjaxCall directly and let it set the value once. What is the use case you're looking at?

#3 @nikeo
3 years ago

@westonruter the purpose of this proposition is to be able to know when all callbacks of an api.Value() instance are done, it's not specifically intended to use the various results of those callbacks, but it can do it.

The use case I had in mind was for sections. For performance improvements, it could be interesting to always defer the instantiation of settings and controls when their sections are expanded.
In this case I think that it would be useful to be able to wait for possible asynchronous content like images to be fetched, before doing other things in the expanded section, like managing the control visibilities interdependencies.

In this case, sections would be bound like this :

api.section( section_id ).expanded.bind( 
    function() {
        //returns a promise()
        return maybe_do_asynchronous_things_to_instantiate_my_controls();
    },
    { deferred : true }
);

And the user click on a section would fire something like this :

api.section( section_id ).expanded( true ).done( function() {
   //do something in this section when all the controls are instantiated and embedded.
   __do_things_when_section_controls_are_all_ready__();
});
Last edited 3 years ago by nikeo (previous) (diff)

#4 @westonruter
3 years ago

@nikeo For the specific use case of lazy-loading things for performance, there is also #28580.

Also, the performance of nav menu is improved specifically by deferring the embedding of nav_menu_item controls until the parent section is expanded. The same is also now being done for widgets. See:
https://core.trac.wordpress.org/browser/tags/4.6.1/src/wp-admin/js/customize-nav-menus.js#L1023
https://core.trac.wordpress.org/browser/tags/4.6.1/src/wp-admin/js/customize-widgets.js#L446

Both of these examples however have all of the controls exported from PHP up-front. There's no lazy-loading yet of the controls. The Customize Posts plugin, however, does lazy-load sections for posts and all of the controls for post fields and postmeta.

I think your use case can currently be satisfied well enough with the current API. Namely:

  1. You expand a section and this triggers an Ajax request to fetch the items (e.g. nav menu items) that would be contained within it.
  2. The Ajax request returns with the list of the items and you generate the IDs for the controls, e.g. nav_menu_item[123].
  3. Given the list of control IDs (controlIds), you create a deferred callback for when they are all created.
  4. Given the created controls, you can pluck out the embedded deferred from each.

In the end, it could look something like this using current customizer JS:

(function ( api, $ ) {
        var fooSectionPopulated, populateFooSectionControls;

        fooSectionPopulated = $.Deferred();

        populateFooSectionControls = function( section ) {
                wp.ajax.send( 'list_bar_items' ).done( function( items ) {
                        var settingIds = [], controlIds = [];
                        /* ... create settings ... */
                        /* ... create controls and set section param to section.id ... */

                        // This isn't really necessary because the controls were just created above.
                        api.control.apply( api.control, controlIds.concat( function() {
                                var controlEmbeddeds = [];
                                _.each( arguments, function( control ) {
                                        controlEmbeddeds.push( control.deferred.embedded );
                                } );

                                $.when( controlEmbeddeds ).done( function() {
                                        fooSectionPopulated.resolve( section );
                                } );
                        } ) )
                } ).fail( function() {
                        fooSectionPopulated.reject();
                } );
        };

        // Lazy-load the controls into section foo when it is expanded.
        api.section( 'foo', function populateSection( section ) {
                var onceExpanded;
                if ( section.expanded() ) {
                        populateFooSectionControls( section );
                } else {
                        onceExpanded = function( isExpanded ) {
                                if ( isExpanded ) {
                                        section.expanded.unbind( onceExpanded );
                                        populateFooSectionControls( section );
                                }
                        };
                        section.expanded.unbind( onceExpanded );
                }
        } );
}) ( wp.customize, jQuery );

Naturally this can be generalized into a more reusable bit of code. I'm not sure wp.customize.Value should be turned into a deferred object itself.

#5 @nikeo
3 years ago

@westonruter thanks for examples and use case.

If we continue with the same example of a section lazy loading its controls, my goal is to do things when the section controls are all embedded ( fooSectionPopulated.done() in your example ).

The question to address is : when is the section expansion actually done ?

  1. Is it when its value has been set ?
  2. Or is it when all its callback have finished their jobs ?

If we consider that a section is expanded as soon as its value is set, then we might have issues using a code like this :

section( section_id )(true);
do_things_with_the_section_controls(); // <= some controls are not ready yet

If we consider that the section is completely expanded when all its callbacks are done, then it make sense to have a way to capture the callbacks promises and to use this syntax :

section( section_id )(true).done( function() {
 do_things_with_the_section_controls(); //<= yes we are sure that all controls are ready because their promises are all resolved
} );

Our approaches differ in the sense that the promise of the section ready ( fooSectionPopulated ) is outside the callback, when I propose that the callback returns the promise.

I agree that there are currently other ways to listen to the Value changes (bind / unbind ). I wanted to propose a complement to the code ( which has no impact if you don't specify the deferred param ), in order to allow different syntaxes when using api.Value() with asynchronous callbacks.

Thanks for the discussion in any cases !

#6 @westonruter
3 years ago

  • Keywords close added

@nikeo I think it is natural to fetch the settings for the controls in a given section upon the expansion of the section, and not before. I mean, if we did this for nav menu items, then we wouldn't know that the settings for nav menu items would need to be fetched. We'd only know to fetch them once the section is expanded. As such, I think there should be a spinner that shows inside of a section while this request is being made, and then when it completes the nav menu item controls would get populated into it. If you wanted to do this the other way around, where you programmatically know up-front that you need to populate a section with controls, then you could do that Ajax request and create all of the settings and controls (as in the populateFooSectionControls example above), and then after completed, expand the section. In either case, however, you'll need a spinner to show that some Ajax request is being done.

I'm thinking this can be closed as maybelater.

#7 @nikeo
3 years ago

@westonruter about the "work in progress" state you describe, I agree, one way or another, the controls / settings lazy loading will require an api state, either assigned to the section instances or api wide.

OK let's do that, this discussion can be flagged "maybelater", it is an idea of implementation among others.

#8 @westonruter
3 years ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.