search

dojo / dojo

Dojo 1 - the Dojo 1 toolkit core library.
https://dojotoolkit.org/
Other
1.56k stars 542 forks source link

Handling DELETE calls as json in JsonRest store. #301

Open ThibaultJanBeyer opened 6 years ago

ThibaultJanBeyer commented 6 years ago

Hi dojo team and thanks for creating this feature rich library.

I saw a little something while using the JsonRest store that could probably be aligned easily:

Query, Add, Put & Get calls all use xhr calls that have the option handleAs: "json" set. But DELETE does not handle the response as json. Which causes the behavior to differ from expectation.

So, please tell me, is that wanted or is it just by mistake?

Do you consider adding the handling for DELETE calls as json like all other calls a valid Pull Request? If so, I would be happy to contribute by creating one.

jsonn commented 6 years ago

Most implementations of DELETE I have seen just confirm the operation, they don't return data. Therefore it is a more useful default.

ThibaultJanBeyer commented 6 years ago

Most, but not all. Some DELETE calls return a confirmation that is used to check against from client side. And since we are in a JsonRest store it makes sense to parse it as json, right?

Or are you saying it would have negative side effects when there is a handleAs: "json" and the call does not return data?

dylans commented 6 years ago

I think there's no clear right answer here. It would be fine to add as an opt-in configuration option, but I don't think we should change the default behavior at this point in time. If there's interest in creating a PR to support this, we would consider it for 1.15.