martyjs / marty

A Javascript library for state management in React applications
http://martyjs.org
MIT License
1.09k stars 76 forks source link

Marty not deserializing requests with application/json headers on certain browser setups #293

Closed nholland94 closed 9 years ago

nholland94 commented 9 years ago

The helper functions that Marty's stateSource api provides for making requests (get, post, etc...) seem to attempt to deserialize json from the response before passing the response back to the success promise whenever the "application/json" headers are present in the response. However, when one of our developers was working on our app, they found that on their Chrome version on their machine, it doesn't happen. It works in other browsers on their machine, just not Chrome. All of our other developers/qa personnel have not run into this. We notice this on the following setup: 64-bit OSX 10.10.2 Chrome version 42.0.2311.135 (64-bit).

For now, we are stuck checking the response object to see if the body is present and calling the json() method manually if it isn't, but it would be nice if we didn't have to do that for all of our json requests.

taion commented 9 years ago

This sounds like it's related to #268. As mentioned it'd probably be best to get rid of the behavior altogether, but there's not a good path to doing so that wouldn't be a (badly) breaking change.

Maybe the workaround in https://github.com/martyjs/marty/commit/27c241d029552d1f6ff8b58b1b38520c91a5657e for that issue is missing some case on Chrome 42.

jhollingworth commented 9 years ago

The parseJSON is causing lots of issues. I think the best thing to do is just remove this hook by default. In v0.10 I will add a deprecation warning and then in v0.11 we won't automatically add it (although you can re-add it if you want).

If you'd like to prepare for that change you can add the following

// Only need to do this once
HttpStateSource.removeHook({ id: 'parseJSON' });

var UsersAPI = Marty.createStateSource({
  type: 'http',
  getUser: function () {
    return this.get('/user').then(function (res) {
      return res.json();
    });
  }
})
taion commented 9 years ago

:+1: It doesn't save much effort to call .body instead of .json().