The Ajax framework was originally intended to be used with FAPI. It was later extended to make it possible to use it with links or other elements.

The current framework is not flexible enough and the code is more complicated than it needs to be. It is also unclear how to use it, some work on the doc wouldn't hurt.

Some practical goals:

  1. Lay down the API and clean it up,
  2. check the implementation of each method of the API,
  3. Validate the way we're dealing with ajax commands on the JS side,
  4. try to simplify and merge certain portion of the code,
  5. find a better place than Drupal.ajax to keep created ajax objects,
  6. BONUS: Investigate if adding basic events and detach makes sense, as described in #1763812: [META] Provide complete attach/detach with basic events

There are other things, feel free to add suggestions.

This is mainly a JS issue, I don't have an opinion on the PHP side of this framework.

Comments

To whomever going to tackle this,
see this module (it is in the patch in linked issue) that makes use of various ajax initializers from php side to trigger front end ajax behaviors: http://drupal.org/node/1473314#comment-5871142 . Useful for testing during ajax.js refactoring. Good luck.

I may have missed something, but it looks like there is not many functions in ajax.js (11, to be exact). And except minor issues, I haven't found anything needing a complete rewrite.

What is there that I completely missed?

I encountered a use case for Drupal.ajax that is not well supported by Drupal.ajax in the Edit module (part of Spark).

Copy/pasted from a chat:

For Edit, I'm currently loading a small form that has just the subform for the field being edited. I do this through a custom #ajax command, which I let then call the insert command, so that it can handle all the behavior attaching and effects stuff. I load my custom AJAX command through Drupal.ajax[some id] = new Drupal.ajax(…) — i.e. the usual pattern.

No problems until here. Even the "add more" button in this form works.

Now there's the case of *removing* this inline form, however… AFAICT there's no clean-up mechanism for Drupal.ajax? Hence, Drupal.ajax[some id] continues to exist. So, whenever I'd close the inline form and load it again, the previous Drupal.ajax would still be there, plus the previous event binding to trigger the AJAX call would also still be there. My current hacks that partially solves this look like this:

   var element_settings = {
     url: url,
     event: 'edit-internal',
     wrapper: 'wat-een-bucht',
   };
   if (Drupal.ajax.hasOwnProperty(edit_id)) {
     delete Drupal.ajax[edit_id];
     $editable.unbind('edit-internal');
   }
   Drupal.ajax[edit_id] = new Drupal.ajax(edit_id, $editable, element_settings);
   $editable.trigger('edit-internal');

Surely there has to be a better way to deal with this?

On top of that, there doesn't seem to be a way to cleanly remove e.g. the Drupal.ajax[…] entry for the "add more" button?

So:

  1. Yes, we very much need this to be a proper JS API, so that modules doing AJAX callbacks can actually use Drupal.ajax. (Why not use jQuery.ajax? Because then you still need to deal with calling the right AJAX commands, animations, etc. — Drupal.ajax does this for you.)
    It also needs to be less "wrapper/id"-centric: maybe you want to use Drupal.ajax just to retrieve custom AJAX commands, for which you don't actually need to specify a wrapper that's going to be replaced/deleted/appended to/whatnot.
  2. The fact that there is *no* clean-up is very bad. This is fine if you only have "fixed" AJAXy DOM elements, but not if you have "dynamic" (as in, they may disappear) AJAXy DOM elements. The latter is the case for me: I need to use AJAX to load forms and those forms may have AJAX too, but those forms may be removed from the DOM after a few seconds.

I'm happy to help on the PHP side (if anything is needed there) and with patch reviews, but need nod_, Wim, or another JS developer to lead the effort here on the JS side.

Issue tags:+WSCCI

Adding WSCCI tag, because to the extent we refactor things here, let's also use it as an opportunity to follow proper REST standards. I don't know yet what that means in detail, but will add links/comments here as I learn, and maybe other WSCCI developers will as well.

Issue tags:-WSCCI

Yep, I'm still thinking about it. We should have a JS sprint/meeting in a month or so where this will come up. Hopefully we'll have a plan after that.

Issue tags:+WSCCI

oups, tag

One potentially major problem I see for this refactoring:

While I believe we have at least some basic test coverage for the fundamental Ajax framework functionality, AFAIK that does not cover all details of the expected functionality, and I also think we do not have many tests for the individual commands. The original patches that introduced the Ajax framework in D7, as well as various follow-ups that fixed regressions and added some tests, never added a complete suite of test coverage.

Technically, refactoring a subsystem can be done relatively easily, if there's a decent amount of test coverage, since that ensures that no expected or required detail in functionality is lost. Without that, very detailed reviews and careful coding/collaboration will be required.

Obviously, we never had frontend/JS tests, so I'm mainly talking about the PHP interaction layer here, for which we extended Simpletest with drupalGetAjax() and drupalPostAjax() methods, which apparently parse the Drupal.settings in the page output and resemble the JS Ajax commands in PHP.

If that can make people feel better I've been all over the AJAX framework as well as pretty deep with ctools ajax JS code, especially the modal commands. I know how it work and I have time to spend on it, that should help things during the refactor.

The ultimate regression tests will come from VDC I guess. But you're right, it might be a good time to go over AJAX testing and see how it can be made better as well, should that be in an other issue ?

Regarding the testing part, if you want to use/extend the testswarm module, I can try to free up some time to help.

Just spotted this via a tag. I cannot speak to the JS side, but from a REST perspective, IMO this should be a custom mime type.

That is, the browser sends an XHR request with mime type application/vnd.drupal-ajax (or something like that; I forget the proper format off hand) to /system/block/$block_id/$node/$user/$whatever. That in turn maps to a controller (nee page callback) that returns a Response object. But not just any response object. It returns a subclass of JsonResponse (probably; maybe just Response, I don't know). That subclass has extra methods on it for us to use that correspond to the various array elements of the current ajax return array. That lets us focus on making that Response class have a really really nice API, and separate out the concerns of the internal data structure (which could be very similar to what we have now, potentially).

Then on send(), that object takes whatever data has been passed to it and parses that down into an appropriate JSON string, then sends that. That JSON string could potentially be exactly the same as what we have now, or it could get cleaned up. I have no preference there.

At least that's how I envisioned it working. Someone else feel free to suggest an alternative, but that's my current thinking there.

The PHP side sounds lovely like that. I'm not sure we have much to clean-up in the json data that is passed around.

The PHP side definitely needs it's own issue too :)

Crell++ :)

Following on from #8 do we need to come up with a list of things we need to test, and get those into Drupal 8 first, and then tackle the refactor?

I'm inclined to say "write the new system, unit test it properly, and throw out the old tests". :-)

I'm willing to try and tackle (parts of) this issue.
If I get this correctly, a complete rewrite on both back and front end is a viable option?

In that case:

  • Are there features that currently are in Drupal AJAX that we would like to see removed?
  • Are there features that currently are in Drupal AJAX that we would like to see improved?
  • Are there features that currently aren't in Drupal AJAX that we would like to see implemented?

I am not an expert, but I think we do want to add history/pushstate support, per: http://groups.drupal.org/node/247073#comment-801003

Definitely we want to improve the error handling, which feels funky at times now.

The internal API is also something we want to improve, which the custom response object should help with by providing real methods rather than anonymous arrays.

Anyone else?

Seems really short-sighted to implement each of the current arrays as a method on a response object, if you go that way then how do I extend the ajax commands with my own? Would it be better to build up a list of objects that represent the commands if we're going to perform? Then I can extend by just creating a new subclass, and adding that to the response object somehow.

You can the current 'API' for the PHP side here:
http://api.drupal.org/api/search/7/ajax_command

Also, it's currently practically impossible to use the AJAX framework with GET requests, #956186: [meta] Allow to use AJAX with GET requests, because of the amount of data being unnecessarily sent by the server. It's not abnormal for the requests to be significantly bigger than the responses at the moment. Ideally we'd get that cleaned up, even if by default most of Drupal core's AJAX request used POST.

From all the links posted here so far I've gathered that the new AJAX implementation should:

  • be easier to understand and use, perhaps a complete rework of the back end
  • iron out the kinks on the front end
  • be extensible on the front end, i.e. fire more events (or use the JS observer pattern)
  • limit sent data to the bare necessities so GET requests are possible
  • be able to style this on the front end, "the D8 way" (think SCOTCH, see #1499460-250: [meta] New theme system)

I've also noted a wish for PJAX, i.e. using the HTML5 History API.
This might not be very practical in combination with forms, though.

Re #22, that seems like a good target list.

Re #20, a more robust framework-esque approach to shuffling commands forward seems sensible, but I worry about complexity and time to implement. Do you have an idea for how that can be done expediently without introducing several additional layers of complication (both for implementers and for people using the API)?

Issue summary:View changes

details

Updated the issue summary (see 'bonus').

So what we have at the moment in AJAX commands is not completely dire, in the case of implementing your own menu callback and returning an AJAX response you'd do something like this in Drupal 7:

<?php
function my_module_ajax_callback() {
 
// An array of commands to return to the client.
 
$commands = array();
 
// An annoying popup alert.
 
$commands[] = ajax_command_alert(t('This is an annoying popup.'));
 
// Replace a div with something else.
 
$commands[] = ajax_command_replace('div#my-div', 'something else');
 
// Return a render array for ajax_deliver()
 
return array('#type' => 'ajax', '#commands' => $commands);
}
?>

But it should be noted that most of the time when dealing with AJAX most people will probably be using forms, and actually you don't really have to do anything different at all then, and you don't have to care about the commands really.

I guess that in an ideal object orientated world then we'd replace the methods with objects, and define an interface something like this (and yes, this would need to be namespaced):

<?php
interface DrupalAJAXCommandInterface {
 
/**
   * Return an array that will be run through json_encode and sent to the client.
   */
 
public function render();
}
?>

Then the above callback would become more like:

<?php
function my_module_ajax_callback() {
 
$response = new DrupalAJAXResponse();
 
// An annoying popup alert.
 
$response->addCommand(new DrupalAJAXCommandAlert(t('This is an annoying popup.')));
 
// Replace a div with something else.
 
$response->addCommand(new DrupalAJAXCommandReplace('div#my-div', 'something else'));
 
// Return a render array for ajax_deliver()
 
return $response;
}
?>

At some later point in the chain, the response is rendered and renders each individual command passed to it, as it does now.

Not sure if that really makes anything clearer, and at some point we still have to render our command down to an array suitable for converting to JSON.

We really should get some feedback from the people originally involved in the AJAX framework, which I think came from ctools?

My initial thinking was to have a fixed list of commands with known methods for each, which is therefore super easy to document and for IDEs to auto-hint. However, I like the idea, in general, of making it extensible with command objects. I believe that's still an improvement over the current status quo, because we can encapsulate cases where we're doing more than just throwing a string into an array. It's also still more self-documenting than an anonymous array is. (I couldn't actually find any documentation in core for the various commands the last time I was looking; as command objects, the docblocks are entirely self-evident.) It also makes it more obvious that you can (presumably) extend it with your own commands. (We'd need to figure out how the client side of that works, of course. Maybe go all hypermedia on it's arse and send the necessary command JS along with the response? Or rather, have no client-side code waiting for it at all, and have the command object just compile down into the code to run on its own, thank you very much, have a nice day.)

As I previously hinted at the current documentation for the ajax commands are here: http://api.drupal.org/api/search/7/ajax_command

Wow, there's more than I thought! Which is itself indicative of something. ;-)

Since we want to OOPify that code anyway so that it can get taken out of the bootstrap process, we should probably get started here. Who wants to take a first crack at it? (Assign to yourself and get cracking!)

I'm still interested in trying to tackle this, but I'm currently tied up in (paid) work.
So if anyone with more time on his hands wants to kick things off already, don't let #18 stop you.

In today's WSCCI meeting, cosmicdreams said he would dig in on this issue to get it bootstrapped.

Sounds like this issue is moving forward :) Thanks to all of you!

I'd be willing to help. I'm very familiar with Drupal's AJAX API, though I'm not quite as familiar with all of the new WSSCI changes. Has there been any bootstrapping of this yet?

Yes please help move this one forward. I haven't been able to do so.

Seems to me (who has admittedly very little context in these discussions) like this #1594870: Decide what KernelEvents::VIEW subscribers should be used for, document it somewhere, and make ViewSubscriber comply might need some doing before the AJAX stuff can be worked on no?

I don't think it's related. VIEW listeners are only called if what the controller returns is not a Response object. If it is a Response object, they are not even called.

If the AjaxResponse class described in #25 is a subclass of Response (or, probably, JsonResponse), then it would do its ow self-compiling in its send() method. None of the View listeners would get called, so ViewSubscriber (which is just a collection of View listeners) would be ignored.

Thank you for stepping up, mkadin! If you have questions, pop into #drupal-wscci in IRC and we'll try to help you out.

I don't know backbone well enough to have a firm opinion, other than to ask "do we really need something that big for a task this small?" It feels like overkill.

OK i've been reading up in an attempt to get this started. If we want to follow through with #12...what are the first steps?

Am I right in saying that we need to set up routing for a custom mime-type to a controller that is specific for our ajax work? If so, I can't seem to figure out how the mime-type routing is implemented.

This one is the JS issue, it needs renaming or taking the PHP side of things in an other issue. I'm happy with the json that's returned for JS to take care of things, I don't have an opinion on the PHP side of this thing but this issue was meant to rewrite the handling and interraction with the ajax framework from the JS side.

Please rename this issue as a meta with an issue summary or open a new one PHP-specific. Thanks.

mkadin: The mime-based routing doesn't exist yet, so for now just have normal controllers that return a Response object, which should skip any of the existing mime-sensitive logic.

I don't much care what issue it's in, so go ahead and spin up a new one if you want. :-)

#1812866: Rebuild the server side AJAX API is a new issue for rebuilding the PHP end of things.

#1812866: Rebuild the server side AJAX API just landed. Yay!

A thought I had recently: Currently we've a lot of code on the client side to take in a bunch of array data from PHP, parse it, map it out to a series of Ajax functions that we are then loading whether it's needed or not (not on every page, but even if you're just using replace you need the whole file), etc. And it's still fairly inscrutable.

What if instead we stripped the API down to bare-bones, and allowed the AjaxCommand object to generate not an array for Drupal.settings but the actual Javascript to execute? So an "insert before" command simply turns into rendering out the one-line jQuery command to run an insert(), which in the browser just gets eval()ed (or whatever the safe alternative is). PHP generating Javascript is not an issue, and this way the browser needs barely any code in it, and processing the commands that come back is a for/eval loop, basically. It also makes the code you have to actually read much much simpler, and you don't need to worry about loading ajax.js. Its replacement gets thin enough that we don't need to break it out to a separate file. Writing a new command gets easier, too, because everything is in one place.

Thoughts?

What you're suggesting is a big eval() on server side generated code?

#42: I have a hard time imagining what that would look like exactly — either your explanation isn't sufficiently clear, or I'm not getting it (very possible — I'm tired). To be honest, it sounds like a JS developer nightmare. It sounds like you intend to move *all* (or as much as possible) logic to the server side. That's already what Drupal.ajax is about today. We need to move it the other way: Drupal.ajax should become easier to use for JS developers.

Wim: Well, right now we do all the JS client side, but control it all server side. That makes both sides gross.

Currently, AlertCommand (which I use as an example because it's the simplest of our available commands) generates a PHP array, which then gets JSON-ified. There's then Javascript code on the client-side (and a non-trivial amount of it for some commands) that takes that array and translates it into an alert() call. Most of the commands are also jQuery-dependent.

I'm suggesting instead that AlertCommand look like this:

<?php
class AlertCommand implements CommandInterface {
  public function
render() {
    return
"alert(" . $this->message . ");";
  }
}
?>

And then the client side is dead-simple code to eval() that. (Again, I don't know client-side standards these days well enough to know if eval() is still as legit client-side as it used to be, or if there's a preferred approach.) Then, writing a server-sent JS command (which you no matter what need server-side code for) consists of "one PHP class in one place that can do whatever it wants", rather than "one PHP class in one place and a Javascript function in another place that you have to make sure is already loaded on the client just in case it may get used, communicating back and forth via arrays that in general seem to presume jQuery although it's not clear if that's a hard requirement". It's probably faster, too.

Drupal.ajax becomes easier for JS devs in that it goes away, and they don't need to try to balance JS and PHP together and keep it all lined up. It's all packaged up in one PHP class; and that doesn't introduce a "need to know PHP" requirement, because to write a new Drupal.ajax command you already have to know PHP, because, well, that's what that system is: Server-side actions that respond with client-side actions.

We can't eval() JS returned from the server like that. Alert is an easy example but it's an example nobody uses. Try having the insert command JS on the PHP side. We use JSON to do the transport and that's great, that's what it's made for. The jQuery dependency is just something that needs to be taken out of commands that don't need it. I agree that we might want to split commands into their own file, and have each command depend on a JS file implementing the command but JS on the PHP side, not going to happen.

The refactor is not about commands which are a good concept with a sensible implementation, it's about the mess that is Drupal.ajax, how to make an Ajax call that is using Drupal commands, calling attachBehaviors when it needs to, etc. Currently you need to have a DOM object and a DOM event to trigger a call. We shouldn't need either to call anything from contrib (or core).

Although it sounds like a good and clean(er) idea at first, passing server generated code into eval() can quickly become a nightmare, both implementation and security wise.

Keep in mind that not all PHP devs know what can and can't be done safely on the frond end. Same goes for JS devs and back end code. I'm afraid we'll unwillingly open the gates for XSS attacks.

How is that any worse than it is now, where writing a new Ajax command requires writing both PHP code and JS code, just in separate files and in more abstracted and indirect form?

Title:Refactor Drupal.ajaxSimplify and optimize Drupal.ajax() instantiation and implementation

So an "insert before" command simply turns into rendering out the one-line jQuery command to run an insert()

The part of this that I agree with is that half of our server-side commands reduce to an 'insert', and I'd be in favor of removing them, and making PHP code just use the Insert command and pass the desired 'method'. Then again, now that we have these commands as Drupal\Core\Ajax\* classes, if we want to leave the specific InsertCommand subclasses around, but maybe change their render() implementation to call parent::render(), or remove the render() override and instead give them a constructor that sets $this->method, we could do that too.

We also have the 'invoke' command for simple 1-line jQuery method invocations, and if we're willing to keep it, then we can drop the 'css' and 'data' commands, unless in #1842576: Review AJAX system commands, decouple from jQuery we decide that we want those commands to be available without jQuery.

What we then have left is a pretty small set of commands: insert, remove, changed, invoke, add_css, settings, restripe, and alert. We can remove 'restripe' if we get #1649780: Remove first/last/odd/even classes in favor of CSS3 pseudo selectors in. I think we might want to replace 'alert' with some more generic way of dealing with errors, warnings, and notices. The remaining commands, though, seem like the right "API" for server-side code to communicate to the client, and I don't think it makes sense to reduce it further. However, we may want to look at other AJAXy frameworks like Backbone to see if they've come up with better or more standard APIs we'd rather model.

Anyway, I agree with #46 that rethinking the command API should happen in #1842576: Review AJAX system commands, decouple from jQuery, and that this issue should focus on the other details of Drupal.ajax(), so retitling this issue. Please feel free to further improve the title as it becomes clearer on what exactly should be done.

Title:Simplify and optimize Drupal.ajax() instantiation and implementationRefactor Drupal.ajax

I agree with the aforementioned arguments, writing actual JS in PHP doesn't sound like a good idea.

We already introduced the invoke() Ajax command, which essentially is a catch-all command to apply any kind of jQuery method on the jQuery object of matched elements, and which can be used for many jQuery plugins without having to write a custom/specialized Ajax command on the PHP side nor the JS side.

The main differentiation however is that the Ajax commands only transfer data to be used as function arguments in JavaScript. They don't attempt to be the actual, executed JS code. (Aforementioned invoke() command borderline crosses that line, but as mentioned, it has a clear purpose and allows one to do really neat things.)

I guess that @merlinofchaos had very good additional reasons for designing the original Ajax framework with this separation, and I could very well imagine that he tried a pure PHP-driven solution first. It would probably make sense to ask him for his experience and reasoning as the original architect of the Ajax framework before discussing this further.

Title:Refactor Drupal.ajaxSimplify and optimize Drupal.ajax() instantiation and implementation

Issue tags:-WSCCI

The PHP issue was split off, tag doesn't make sense for the new scope.

More or less I agree with nod.

Changing how the commands are processed isn't really going to benefit, I don't think; fixing up Drupal.ajax to be simpler and easier to deal with would be important. Drupal.ajax is a merger of my original AJAX code from CTools Drupal 6, which didn't have that kind of a construct, and ahah.js from Drupal 6 core, which did.

To elaborate, these are my reasons for the separation:

1) I don't really like eval() to begin with, and didn't feel confident I understood the security implications. Whereas, a simplified "macro language" which is what I think of, is much safer in general.
2) The AJAX commands, at least the non-trivial ones, can need context, so if you're writing complex code, you're more or less going to have to do what is done now anyway, only you'll have to eval something that calls to your object. This would likely be more brittle than what we have now.
3) Debugging eval() processed from JSON is much harder than debugging Drupal.AJAX.command.foo with input data.

from #1810934: Adding custom ajax progress indicators more easily

Maybe we could improve the way(s) developers can implement custom progress indicators.

What do you think about the following code which is a refactored version of the current 8.x-dev code:

Drupal.ajax.prototype.beforeSend = function (xmlhttprequest, options) {
  // ...
  // Insert progress indicator
  var progressIndicatorMethod = 'setProgressIndicator' + this.progress.type.slice(0,1).toUpperCase() + this.progress.type.slice(1).toLowerCase();
  if (this[progressIndicatorMethod] && this[progressIndicatorMethod].call) {
    this[progressIndicatorMethod].call(this);
    $(this.element).after(this.progress.element);
  }
};

So we simply factor out the method for the existing progress indicators, having the side effect of allowing custom progress indicators:

Drupal.ajax.prototype.setProgressIndicatorProgressbar = function() {
  var progressBar = new Drupal.progressBar('ajax-progress-' + this.element.id, eval(this.progress.update_callback), this.progress.method, eval(this.progress.error_callback));
  if (this.progress.message) {
    progressBar.setProgress(-1, this.progress.message);
  }
  if (this.progress.url) {
    progressBar.startMonitoring(this.progress.url, this.progress.interval || 1500);
  }
  this.progress.element = $(progressBar.element).addClass('ajax-progress ajax-progress-bar');
  this.progress.object = progressBar;
};

Drupal.ajax.prototype.setProgressIndicatorThrobber = function() {
  this.progress.element = $('<div class="ajax-progress ajax-progress-throbber"><div class="throbber">&nbsp;</div></div>');
  if (this.progress.message) {
    $('.throbber', this.progress.element).after('<div class="message">' + this.progress.message + '</div>');
  }
  $(this.element).after(this.progress.element);
};

This way developers can use their custom progress indicators simply by using:

<?php
  $form
= array(
   
'element' => array(
     
'#type' => 'textfield',
     
'#ajax' => array(
       
'callback' => 'mymodule_ajax_callback',
       
'wrapper' => 'ajax-wrapper',
       
'progress' => array(
         
'type' => 'custom'
         
// Additional custom options
       
),
      ),
    ),
  );
?>

And then implementing:

Drupal.ajax.prototype.setProgressIndicatorCustom = function() {
  // Setting this.progress.element is the only necessary thing to happen in here.
  this.progress.element = $('<div class="ajax-progress ajax-progress-custom">indicator</div>');
};

Priority:Normal» Major

I know it's past API freeze and all but we can't make thousands of developers miserable because of Drupal.ajax in D8 cycle. Raising priority.

Issue summary:View changes

detach/events bonus point