I think this is a bit strange/confusing:

       // Use a dummy url.
       var ajaxObject = Drupal.ajax({url: 'big-pipe/placeholder.json'});

I think we can do better?

CommentFileSizeAuthor
#7 2670426-7.patch821 bytesWim Leers
#2 2670426-2.patch749 bytesWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Assigned: Wim Leers » Unassigned
Status: Active » Needs review
FileSize
749 bytes

This is equivalent, clearer, simpler.

Wim Leers’s picture

I'd like to get @nod_'s approval for this.

nod_’s picture

I mean if we're there why not be even more direct:


var commands = new Drupal.AjaxCommands();
var dummyAjax = {getEffect: Drupal.Ajax.prototype.getEffect}; 
response.forEach(function (item) {
  if (commands[item.command]) {
    commands[item.command](dummyAjax, item);
  }
});

And we should get rid of the hard requirement on getEffect in core so we don't need the dummy function.

Wim Leers’s picture

Oh, interesting!

I personally find my proposal easier to grok, because it makes it very clear that:

  1. the content we've just parsed is just another Ajax response, so we reuse success()
  2. it is associated with no element, has no settings, doesn't use a progress indicator and … has no URL, which makes total sense given it's literally an embedded AJAX response, as the code docs already say

Thoughts?

nod_’s picture

I'd rather not have people use Drupal.Ajax directly, and nobody knows what the parameters of Drupal.Ajax() are so having false, false doesn't help make it clearer. Whereas using Drupal.ajax you can do

Drupal.ajax({
  url: '',
  // The rest is done by default but you can be extra explicit for clarity.
  base: false,
  element: false,
  progress: false,
});

Wim Leers’s picture

FileSize
821 bytes

Alright, so then what about this, which is exactly the same as HEAD, but is just more explicit per #6 and includes the docs of #2.

nod_’s picture

All good :)

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

  • Wim Leers committed db8be04 on 8.x-1.x
    Issue #2670426 by Wim Leers, nod_: Improve BigPipe's use of ajax.js
    
Wim Leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.