Bug Tracker

Opened 8 years ago

Closed 5 years ago

#9885 closed feature (migrated)

Add global Ajax beforeSend event

Reported by: joshpeek Owned by: dmethvin
Priority: low Milestone: None
Component: ajax Version: 1.6.2
Keywords: 1.9-discuss Cc: jaubourg
Blocked by: Blocking:

Description

Adds global ajaxBeforeSend to complement local beforeSend event.

https://github.com/jquery/jquery/pull/439

Change History (21)

comment:1 Changed 8 years ago by dmethvin

Global events are destined for the trash heap if I have any say. I think our long-term goal is to remove them and trigger ajax events only on document. That's a breaking change of course, so it needs to be done with a lot of advance notice.

It's already possible to hook beforeSend via ajaxSettings, does that suffice for now?

comment:2 Changed 8 years ago by jaubourg

Resolution: wontfix
Status: newclosed

Like Dave said, ajax events are on their way to deprecation. But you actually don't need a beforeSend event at all for the use cases you have as examples in your pull request: use ajax prefilters.

For your first use case:

$.ajaxPrefilter(function( settings ) {
    if ( settings.dataTypes[ 0 ] === "*" ){
        xhr.setRequestHeader( "accept", "*/*;q=0.5, " +
            settings.accepts.script);
    }
});

For your second use case:

$.ajaxPrefilter(function( settings, _, jqXHR ) {
    if ( settings.validateForm &&
            !validateForm( settings.context ) ) {
        jqXHR.abort();
    }
});

In that case, you just have to add the validateForm option in all ajax calls that related to a form that needs validation (and put the form as context, of course).

comment:3 Changed 8 years ago by jaubourg

Component: unfiledajax

comment:4 Changed 8 years ago by jaubourg

Cc: jaubourg added

comment:5 Changed 7 years ago by joshpeek

It sounds like removing ajax global events, was just to deprecate global event triggers rather than kill the events all together. I'm definitely in favor of just triggering them off the document.

I've been living with a ajaxBeforeSend polyfill for a while now. Yeah, you just hook it onto $.ajaxSetup. The issue is thats any one who passes their own beforeSend callback to $.ajax will override it and disable the event. This is the major issue with $.ajaxSetup in general. Global ajax events are a nice alternative to $.ajaxSetup. I don't think I've ever wanted my global callbacks to be disabled if a local one is provided.

I really don't think you want to be using $.ajaxPrefilter for domain specific form validation. Its an awful api for that. The wonderful thing about events is that they are easily cancelable and have bubbling semantics. You can't use any of the event delegation machinery on prefilters. "this" in prefilters is also a meanings global context.

The first use case pointed out is kinda what prefilters we're design for. But you could still accomplish the same thing either way.

The second use case is not quite the same since it requires you to set a settings.validateForm. The real equivalent would be

$.ajaxPrefilter(function(settings) {
  if ($(settings.context).is('form') && !validateForm(settings.context))
    jqXHR.abort();
});

Not having to write out element matchers like that is why using $.fn.on is so much nicer than $.fn.bind.

Thats kinda a more generic example, but magnify the situation when you try to use this many times in domain specific contexts.

$(document).on('ajaxBeforeSend', 'form.new-comment[data-remote]', function() {
  return $(this).find('input').val() != '';
})

They also pair really great with ajaxComplete.

$(document).on('ajaxBeforeSend', function() { $(this).addClass('loading') })
$(document).on('ajaxComplete', function() { $(this).removeClass('loading') })

See Also

  • Rails' jquery adapter https://github.com/rails/jquery-rails a bubbling beforeSend event in the form of ajax:beforeSend. So everyone using Rails already has this when interacting with data-remote forms. With data-remote you can't set stuff like settings.validateForm because you don't control the $.ajax's caller. You're forced to be a delegate of the action which means the ajax content settings matters so much more.
  • Zepto is already doing this https://github.com/madrobby/zepto/blob/master/src/ajax.js#L40-48. They're not implementing the prefilter api because this was only 3 lines of code to add.

What do prefilters offer that you can't do with a simple event? The related prefilter code is like 70 lines. And you guys already have a pretty great event system.

https://github.com/jquery/jquery/blob/master/src/ajax.js#L62-136

comment:6 Changed 7 years ago by dmethvin

Resolution: wontfix
Status: closedreopened

I think you've argued the case well, and the cost to implement isn't high if we're just firing the event on document. @jaubourg, what do you think?

comment:7 Changed 7 years ago by jaubourg

What I think I'm on the road with a choppy choppy network connection and that I can't answer right now but I respectfully disagree with joshpeek. I cannot, however, give a proper answer before next week, when I'll have enough time to do it properly.

comment:8 Changed 7 years ago by scottgonzalez

For what it's worth, this has bothered me for years. $.ajaxSetup is annoying and having a global beforeSend is useful. When I wrote amplify.request I made sure to publish a message from the beforeSend callback. I have never understood why this isn't a one-to-one correspondence between callbacks and events for ajax requests.

comment:9 Changed 7 years ago by dmethvin

Keywords: 1.9-discuss added
Status: reopenedopen

comment:10 Changed 7 years ago by dmethvin

Type: enhancementfeature

Bulk change from enhancement to feature.

comment:11 Changed 7 years ago by dmethvin

+1, jaubourg didn't complete his thought there but it seems like it should be there for completeness. Only fired on document.

comment:12 Changed 7 years ago by mikesherov

+1, same sentiments as Dave.

comment:13 Changed 7 years ago by Rick Waldron

+1, only for consistency

comment:14 Changed 7 years ago by timmywil

Priority: undecidedlow

comment:15 Changed 7 years ago by timmywil

+1

comment:16 Changed 7 years ago by mikesherov

Milestone: None1.9

comment:17 Changed 7 years ago by dmethvin

Owner: set to dmethvin
Status: openassigned

comment:18 Changed 7 years ago by jaubourg

It's still -1 for me.

The only issue the OP expressed regarding a prefilter is that he needs to use settings.context explicitely. As far as I understand it, this is now also the case in global events now that they only fire on document. Also, a prefilter doesn't incur a performance penalty in the order of magnitude firing an event does. A prefilter also cannot be accidently removed by third party code (document.off("ajaxBeforeSend") anyone?).

It also automagically offers a closure where data can be shared across handlers (the prefilter itself, done and fail) without the need to attach data to the context itself, data that pertains to the ajax request, not the DOM element. In short, it's a self-contained space that is ideal for this kind of problems and should be promoted as such.

Introducing a global ajaxBeforeSend is yet another step in a direction we know is wrong and the need for orthogonal logic that has to run before a request is sent is much better covered by prefilters. That's the reason they exist in the first place.

comment:19 Changed 7 years ago by dmethvin

Milestone: 1.9None

comment:20 Changed 6 years ago by joshpeek

Could this be reconsidered now that global events fire off document.

comment:21 Changed 5 years ago by m_gol

Resolution: migrated
Status: assignedclosed
Note: See TracTickets for help on using tickets.