Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

[ENHANCEMENT] Don't make assumptions about dataType and payload. #5

Merged
merged 1 commit into from
Sep 25, 2015

Conversation

abuiles
Copy link
Member

@abuiles abuiles commented Sep 25, 2015

Since the final goal of this is to become a service for making AJAX request, it would be a good idea not to make many assumptions about such request, AJAX does not necessarily mean json.

I'd like to propose we just expose the basic interface for jQuery.ajax without trying to be too smart about it. With this we also address issues like #2.

Also what about changing the the args of service.request to be ONLY url and options, I think this provides better ergonomics that the current implementation, for example if I want to do a get with query params I'd have to do something like service.request(url, 'GET', {data: {foo: true}} this could be reduce to service.request(url, {data: {foo: true}} since GET tends to be the default, and then if I need a different request type, then I could just specify that in the options (like $.ajax)

@taras
Copy link
Contributor

taras commented Sep 25, 2015

Great simple solution. Thank you

taras added a commit that referenced this pull request Sep 25, 2015
[ENHANCEMENT] Don't make assumptions about dataType and payload.
@taras taras merged commit 8bcf9b6 into master Sep 25, 2015
@taras taras deleted the be-more-flexible branch September 25, 2015 16:42
@abuiles
Copy link
Member Author

abuiles commented Sep 25, 2015

@taras I'll open a second PR for the argument change, notice they are breaking changes.

@taras
Copy link
Contributor

taras commented Sep 25, 2015

Also what about changing the the args of service.request to be ONLY url and options, I think this provides better ergonomics that the current implementation, for example if I want to do a get with query params I'd have to do something like service.request(url, 'GET', {data: {foo: true}} this could be reduce to service.request(url, {data: {foo: true}} since GET tends to be the default, and then if I need a different request type, then I could just specify that in the options (like $.ajax)

I agree. I think is a good idea. Can we make it backwards compatible?

@devinus
Copy link

devinus commented Dec 22, 2015

This would have been great if it then checked for dataType here 63973c3#diff-6ad1f65c87cc1996c63b65a765d84933L131 rather than removing it.

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

Successfully merging this pull request may close these issues.

None yet

3 participants