Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default queue name to request URL if not provided #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elucid
Copy link

@elucid elucid commented Jan 19, 2013

This is a great plugin. I've used it on a number of projects and noticed that in all but one case I used the request URL as the queue name. To me, this seems like a reasonable default for scoping requests.

This simple patch does just that. If no queue name is provided, it uses the request URL. Now adding support for queued requests is as simple as including the library and changing calls from $.ajax to $.ajaxq.

@bgrins
Copy link
Contributor

bgrins commented Jan 22, 2013

Thanks for sending this in! I think that providing a default could make it even easier to use, and the URL seems reasonable. What do you think about also tacking on the method onto the default queue name? I'd like to make it so that the default could be overridden by someone using the plugin, and also like to distinguish between GET and POST requests to the same URL. What do you think about something like this:

$.ajaxq.getDefaultQueueName = function(opts) {
    return (opts.type || "GET") + "-" + opts.url;
};

Which would be called similarly to your pull request, like this:

if (typeof opts === "undefined") {
    opts = qname;
    qname = $.ajaxq.getDefaultQueueName(opts);
}

@elucid
Copy link
Author

elucid commented Jan 22, 2013

Hey Brian,

Thanks for the feedback. I like the idea of an overridable default queue name function even better. I can modify my PR to include that.

However, I'm not sure about including the method in the queue name by default. Maybe I'm missing something, but I don't understand why one would want to queue GET requests to a specific resource except to avoid fetching the resource before pending updates are processed. If that is the case, then including the method in the queue name would actually break this behaviour.

Is there another use case for queued GET requests that I am not thinking about?

@bgrins
Copy link
Contributor

bgrins commented Jan 22, 2013

The idea was to prevent unintentional queuing. If you specifically didn't want a GET request to be queued, you would be using $.ajax anyway, and if you wanted to queue a GET behind a POST, then you could always just pass in a queue name as before.

That said, I see where you are coming from - it seems like a weird use case where the you would want GETs to be queued separately from a POST. Did you rely on the queue including GET requests in your implementations, or were you only using it for POSTs?

My reasoning was to prevent a confusing issue where you aren't expecting something to get put in the same queue as your other requests. I think that could be caused by a poorly designed API that shared a URL and overloaded the action that happens on it based on a value passed up, for instance. See a made up example below. If you are doing this though, it is likely that no default queue names will cut it even if we included the method.

$.ajaxq( {
    url: "/doaction",
    type: "POST",
    data: {
        action: "addElement",
        name: "New Element"
    }
});

$.ajaxq( {
    url: "/doaction",
    type: "GET",
    data: {
        action: "heartbeat"
    }
});

$.ajaxq( {
    url: "/doaction",
    type: "GET",
    data: {
        action: "heartbeat"
    }
});

@bgrins
Copy link
Contributor

bgrins commented Jan 22, 2013

I'm OK with not including method if you feel strongly about it - I haven't used the default queue behavior, so it was just speculating that method would be an important thing to add on.

Do you have any thoughts about handling getq and postq with no values? Seems like it may be tricky with all the potential overloads to the function already. I'm OK with leaving those out if we can't think of a simple way to handle that.

@elucid
Copy link
Author

elucid commented Jan 23, 2013

I would expect queueing GET requests to be rather rare. I haven't used queued GET requests in any of my apps. The advantage you get from queuing POSTs and PUTs is that you avoid issues with out of order updates. I can see that queued GETs might be useful if you were waiting on some POST or PUT to complete, but then again you would probably do that within the success callback for the appropriate update request. In your example above, I would argue that using $.ajaxq for the GET requests is actually a mistake and that plain $.ajax should be used instead.

Anyway, I agree with your point that badly designed APIs will unlikely be able to take advantage of default queue names, regardless of how the are generated. As a result I think it makes sense to have the default cater to nice APIs and just base the queue name on the request URL. If you are okay with this I will add some code to the PR to reflect the new default queue name function. I will also add some documentation.

As for the question as to what should be done about $.getq and $.postq--I'm not sure. I will have to take a look at the jQuery docs a bit more carefully and see if I can think of something reasonable to do.

@bgrins
Copy link
Contributor

bgrins commented Jan 24, 2013

Sounds like a good plan. Let's just go with URL for now, and if you can add some documentation about it that would be great. I think getq and postq are going to be tricky just looking at the param ordering. We should just add a note that you need to specify a queue name if you use those.

@elucid
Copy link
Author

elucid commented Jan 25, 2013

Okay. I'll update the PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants