Merge pull request #70 from marmelab/firefox_fixes

[RFR] Fix firefox issue with headers.forEach and make type check optional

build/restful.fetch.js

@@ -2,6 +2,6 @@ import restful from '../src';
 import fetchBackend from '../src/http/fetch';
 import 'whatwg-fetch';
 
-export default function(baseUrl, httpBackend = fetchBackend(fetch)) {
+export default function (baseUrl, httpBackend = fetchBackend(fetch)) {
     return restful(baseUrl, httpBackend);
 }

package.json

@@ -16,24 +16,25 @@
   "author": "Robin Bressan <[email protected]>",
   "license": "MIT",
   "dependencies": {
-    "immutable": "~3.7.5",
-    "object-assign": "~2.0.0",
-    "qs": "~5.2.0"
+    "immutable": "3.7.6",
+    "object-assign": "2.0.0",
+    "qs": "5.2.0",
+    "warning": "2.1.0"
   },
   "devDependencies": {
-    "babel": "~5.8.23",
-    "babel-loader": "~5.3.2",
-    "babel-eslint": "~4.0.10",
-    "chai": "~3.2.0",
-    "eslint": "~1.5.1",
-    "eslint-config-airbnb": "~0.1.0",
-    "mocha": "~2.3.0",
-    "nock": "~2.12.0",
-    "sinon": "~1.16.1",
-    "webpack": "~1.9.11"
+    "babel": "5.8.35",
+    "babel-loader": "5.3.3",
+    "babel-eslint": "4.0.10",
+    "chai": "3.5.0",
+    "eslint": "1.5.1",
+    "eslint-config-airbnb": "0.1.0",
+    "mocha": "2.3.4",
+    "nock": "2.12.0",
+    "sinon": "1.17.3",
+    "webpack": "1.12.12"
   },
   "optionnalDependencies": {
-    "request": "~2.62.0",
-    "whatwg-fetch": "~0.9.0"
+    "request": "2.69.0",
+    "whatwg-fetch": "0.11.0"
   }
 }

src/http/fetch.js

@@ -1,6 +1,24 @@
 import qs from 'qs';
+import warning from 'warning';
 
-export default function(fetch) {
+function parseBody(response) {
+    return response.text().then(text => {
+        if (!text || !text.length) {
+            warning(response.status === 204, 'You should return a 204 status code with an empty body.');
+            return null;
+        }
+
+        warning(response.status !== 204, 'You should return an empty body with a 204 status code.');
+
+        try {
+            return JSON.parse(text);
+        } catch (error) {
+            return text;
+        }
+    });
+}
+
+export default function (fetch) {
     return (config) => {
         const url = config.url;
         delete config.url;
@@ -15,16 +33,17 @@ export default function(fetch) {
 
         return fetch(!queryString.length ? url : `${url}?${queryString}`, config)
             .then((response) => {
-                return (response.status === 204 ? Promise.resolve(null) : response.json()).then((json) => {
+                return parseBody(response).then((json) => {
                     const headers = {};
-
-                    response.headers.forEach((value, name) => {
-                        headers[name] = value;
-                    });
+                    const keys = response.headers.keys();
+                    for (const key of keys) {
+                        headers[key] = response.headers.get(key);
+                    }
 
                     const responsePayload = {
                         data: json,
                         headers: headers,
+                        method: config.method ? config.method.toLowerCase() : 'get',
                         statusCode: response.status,
                     };
 

src/http/request.js

@@ -1,4 +1,4 @@
-export default function(request) {
+export default function (request) {
     return (config) => {
         if (config.data) {
             config.form = /application\/json/.test(config.headers['Content-Type']) ? JSON.stringify(config.data) : config.data;

src/model/decorator.js

@@ -12,7 +12,7 @@ export function custom(endpoint) {
 
 export function collection(endpoint) {
     function _bindHttpMethod(method) {
-        return (...args)  => {
+        return (...args) => {
             const id = args.shift();
             return member(endpoint.new(`${endpoint.url()}/${id}`))[method](...args);  // eslint-disable-line no-use-before-define
         };
@@ -24,7 +24,6 @@ export function collection(endpoint) {
         getAll: endpoint.get,
         get: _bindHttpMethod('get'),
         head: _bindHttpMethod('head'),
-        one: (name, id) => member(endpoint.new(`${endpoint.url()}/${name}/${id}`)), // eslint-disable-line no-use-before-define
         patch: _bindHttpMethod('patch'),
         put: _bindHttpMethod('put'),
     });

src/model/response.js

@@ -1,9 +1,10 @@
 import entity from './entity';
 import { List } from 'immutable';
 import serialize from '../util/serialize';
+import warning from 'warning';
 
 /* eslint-disable new-cap */
-export default function(response, decoratedEndpoint) {
+export default function (response, decoratedEndpoint) {
     const identifier = decoratedEndpoint.identifier();
 
     return {
@@ -18,19 +19,15 @@ export default function(response, decoratedEndpoint) {
             }
 
             if (List.isList(data)) {
-                if (decoratedEndpoint.all) {
-                    throw new Error('Unexpected array as response, you should use all method for that');
-                }
+                warning(response.get('method') !== 'get' || !decoratedEndpoint.all, 'Unexpected array as response, you should use all method for that');
 
                 return serialize(data.map((datum) => {
                     const id = datum.get(identifier);
                     return entity(serialize(datum), decoratedEndpoint.custom(`${id}`));
                 }));
             }
 
-            if (!decoratedEndpoint.all) {
-                throw new Error('Expected array as response, you should use one method for that');
-            }
+            warning(response.get('method') !== 'get' || decoratedEndpoint.all, 'Expected array as response, you should use one method for that');
 
             return entity(serialize(data), decoratedEndpoint);
         },

test/mock/api.js

@@ -1,7 +1,7 @@
 import articles from '../fixture/articles';
 import comments from '../fixture/comments';
 
-export default function(nock) {
+export default function (nock) {
     const server = nock('http://localhost');
 
     server

test/src/http/fetchSpec.js

@@ -10,11 +10,10 @@ describe('Fetch HTTP Backend', () => {
     beforeEach(() => {
         response = {
             headers: {
-                forEach: (cb) => {
-                    cb('here', 'test');
-                },
+                keys: sinon.stub().returns(['test']),
+                get: sinon.stub().returns('here'),
             },
-            json: sinon.stub().returns(Promise.resolve({ content: 'Yes' })),
+            text: sinon.stub().returns(Promise.resolve(JSON.stringify({ content: 'Yes' }))),
             status: 200,
         };
         fetch = sinon.stub().returns(Promise.resolve(response));
@@ -88,6 +87,7 @@ describe('Fetch HTTP Backend', () => {
                 headers: {
                     test: 'here',
                 },
+                method: 'get',
                 statusCode: 200,
             });
 
@@ -112,14 +112,17 @@ describe('Fetch HTTP Backend', () => {
         })
         .then((_response) => {
             expect(_response).to.deep.equal({
-                data: null,
+                data: {
+                    content: 'Yes',
+                },
                 headers: {
                     test: 'here',
                 },
+                method: 'get',
                 statusCode: 204,
             });
 
-            expect(response.json.callCount).to.equal(0);
+            expect(response.text.callCount).to.equal(1);
             done();
         })
         .catch(done);
@@ -150,6 +153,7 @@ describe('Fetch HTTP Backend', () => {
                 headers: {
                     test: 'here',
                 },
+                method: 'get',
                 statusCode: 404,
             });
 

test/src/model/decoratorSpec.js

@@ -29,7 +29,7 @@ describe('Decorator Model', () => {
             expect(endpoint.new.getCall(0).args).to.deep.equal([
                 '/url/articles/1',
             ]);
-            expect(childMember.fake).to.be.true;
+            expect(childMember.fake).to.equal(true);
             expect(typeof(childMember.all)).to.equal('function');
             expect(typeof(childMember.custom)).to.equal('function');
             expect(typeof(childMember.one)).to.equal('function');
@@ -44,8 +44,7 @@ describe('Decorator Model', () => {
                 '/url/articles',
             ]);
 
-            expect(typeof(articles.one)).to.equal('function');
-            expect(articles.all).to.be.undefined;
+            expect(articles.all).to.equal(undefined);
             expect(typeof(articles.custom)).to.equal('function');
         });
 
@@ -78,23 +77,7 @@ describe('Decorator Model', () => {
         it('should add custom and one methods to an endpoint', () => {
             const collection = decorators.collection(endpoint);
 
-            expect(collection.all).to.be.undefined;
             expect(typeof(collection.custom)).to.equal('function');
-            expect(typeof(collection.one)).to.equal('function');
-        });
-
-        it('should create a new endpoint with correct url when one is called', () => {
-            const collection = decorators.collection(endpoint);
-
-            const childMember = collection.one('articles', 1);
-
-            expect(endpoint.new.getCall(0).args).to.deep.equal([
-                '/url/articles/1',
-            ]);
-            expect(childMember.fake).to.be.true;
-            expect(typeof(childMember.all)).to.equal('function');
-            expect(typeof(childMember.custom)).to.equal('function');
-            expect(typeof(childMember.one)).to.equal('function');
         });
 
         it('should create a new endpoint with correct url when custom is called', () => {

webpack.config.js

@@ -18,7 +18,7 @@ module.exports = {
         new webpack.DefinePlugin({
             'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV),
         }),
-    ].concat(production ?  [
+    ].concat(production ? [
         new webpack.optimize.UglifyJsPlugin(),
         new webpack.optimize.DedupePlugin(),
     ] : []),