ampersand-sync
ampersand-sync copied to clipboard
Allow the HTTP method to be provided directly
This PR will allow the ampersand-sync's callee to provide the HTTP method to use in the call directly (instead of forcing you to pass the model's intention).
In REST systems, the need to call custom endpoints is a common one (Eg, sending a POST request to some ...api/:id/refresh). In order to accommodate for this, a good approach would be to implement a custom method on the model, or a more generic function such as: https://gist.github.com/ruiramos/1687be7a7edaa14816ec
This PR intends to remove the need for that unMapMethod
map and make ampersand-sync a bit more generic and less dependent on model lingo.
Makes sense to me, but I'd like to see some updated tests included with the PR.
e.g. What happens if you pass in a bad method
?
Method can be provided directly in options that are passed to xhr.
sync(whatever,model, { method: "POST" })
should do.
I don't see why making the first argument ambiguous is beneficial to any usecase. Could you provide your usecase? Maybe you just wanted to invoke xhr
directly?
Hey @naugtur and thanks for your inputs. I think the use case is pretty clear in the description/gist linked: to be able to extend a model with custom RESTful requests, based on its endpoint.
It's true that xhr
could be directly used in the model, or the first parameter simply discarded and overwritten by options
, but these doesn't seem better options to me as you lose the common interface and all the extra features sync is supposed to give you -- like triggering request
on model, the ability to build query strings on GETs, emulate JSON, etc.
I'm a +1 with tests as per @fyockm's suggestion.
@ruiramos makes sense to keep the api, I'm just worried it's going to be hard to explain. And sorry I didn't look at the gist, I just jumped in to check if it's something that the options were for, and I still don't see how handling this as a first argument is significantly better.