search

mikehostetler / amplify

AmplifyJS
http://amplifyjs.com
GNU General Public License v2.0
1.45k stars 143 forks source link

Document all requests are now asynchronous #29

Closed jdsharp closed 12 years ago

eliperelman commented 12 years ago

Can I ask why all requests are async now?

scottgonzalez commented 12 years ago

Because it's really bad to have an API that is sometimes synchronous and sometimes asynchronous. Since amplify.request is an abstraction that will generally be asynchronous, it should not be possible to create a synchronous request.

eliperelman commented 12 years ago

OK. Has any kind of consensus been made on the type of promises that are going to be used in amplify.request? I just wonder because if request has a promise interface then you wouldn't have to force syncs to async, since obviously they share the same API.

scottgonzalez commented 12 years ago

Not yet, and there's actually nothing about a promise that enforces async resolution.

eliperelman commented 12 years ago

Agreed, but why not allow sync and async? jQuery's promise API allows either, so for some reason they believe it is OK for a common API to be sync or async. I'm just wondering the rationale behind the idea that sync and async shouldn't share the same API...

scottgonzalez commented 12 years ago

Mixing sync and async creates undefined behavior and prevents you from writing performance-optimized code based on the fact that you're starting an async action. If you want an in-depth explanation, you might find one somewhere in a node.js mailing list thread, where all things async get discussed ad nauseam. Allowing both also breaks the abstraction that amplify.request provides because it means that changing the definition can change the behavior of the calling code.

jQuery's promises support synchronous resolution in order to support synchronous ajax requests, which are pretty bad. This is really annoying, because it means that even an asynchronous ajax request can resolve synchronously (in the case of IE having an HTTP cache).

eliperelman commented 12 years ago

Ah, I see. So it's being done to improve performance and not because of API inconsistencies. I am definitely in favor of that. Thanks for the insight Scott.

scottgonzalez commented 12 years ago

It's for both, API consistency is extremely important. Also, it's not for performance inside of amplify, it's for ensuring that you can write performant code with the knowledge that the request will be async. This is actually a performance hit inside of amplify.

eliperelman commented 12 years ago

Understood. The reason I asked this question was I was looking through the request source trying to see what it would take to convert it to use a promise interface, and I ran into the async method. I know what it does, I was just trying to understand why amplify thought it was the best approach to take. I am agreeing with your performance comment; I also meant for calling code, not amplify itself.