Deprecate 'data' for 'body', and respect existing query strings (#11)

* Deprecate 'data' in favor of 'body'

* Fix 'query' to use existing query string if present

* Linting

* Add tests for deprecated data option

* Upgrade deps

* Color the deprecation warning yellow

.travis.yml

@@ -1,10 +1,10 @@
 language: node_js
 node_js:
+  - '10'
   - '8'
-  - '7'
   - '6'
 before_install:
-  - npm install -g yarn@0.27.5
+  - npm install -g yarn@1.10.0
 install:
   - yarn install --pure-lockfile
 script:

API.md

@@ -19,7 +19,7 @@ The following params are accepted:
 
 | Name | Type | Default | Description |
 | ---- | ---- | ------- | ----------- |
-| `data` | `Any` | | data to serialize and send with the request |
+| `body` | `Any` | | data to serialize and send as the request body |
 | `deserialize` | `Function` | [`JSON.stringify`](http://devdocs.io/javascript/global_objects/json/stringify) | function with which to deserialize the response body |
 | `headers` | `Object` | `{}` | headers to include on the request |
 | `json` | `Boolean` | `true` | if `true`, assumes [json-formatted](#json-by-default) request and response |

README.md

@@ -2,8 +2,17 @@
 [![@articulate/gimme](https://img.shields.io/npm/v/@articulate/gimme.svg)](https://www.npmjs.com/package/@articulate/gimme)
 [![Build Status](https://travis-ci.org/articulate/gimme.svg?branch=master)](https://travis-ci.org/articulate/gimme)
 [![Coverage Status](https://coveralls.io/repos/github/articulate/gimme/badge.svg?branch=master)](https://coveralls.io/github/articulate/gimme?branch=master)
-[![NSP Status](https://nodesecurity.io/orgs/articulate/projects/72be8ca8-9eaa-4e1e-a824-e79532450652/badge)](https://nodesecurity.io/orgs/articulate/projects/72be8ca8-9eaa-4e1e-a824-e79532450652)
 
 Rest client that goes :boom:
 
-See the [documentation](https://github.com/articulate/gimme/blob/master/API.md) for details and examples.
+## Usage
+
+```haskell
+gimme : { k: v } -> Promise Boom Response
+```
+
+Accepts an object of request params, described below.
+
+Returns a [`Promise`](http://devdocs.io/javascript/global_objects/promise) that either resolves with a [`Response`](#response-object), or rejects with an appropriate [`Boom`](https://www.npmjs.com/package/boom) error for the status code of the response.
+
+See the [documentation](https://github.com/articulate/gimme/blob/master/API.md) for more details and examples.

index.js

@@ -6,7 +6,8 @@ const { validate } = require('@articulate/funky')
 const gimme = require('./lib/gimme')
 
 const schema = Joi.object({
-  data:        Joi.any(),
+  body:        Joi.any(),
+  data:        Joi.any(), // deprecated
   deserialize: Joi.func(),
   headers:     Joi.object(),
   json:        Joi.boolean(),
@@ -15,7 +16,7 @@ const schema = Joi.object({
   serialize:   Joi.func(),
   stream:      Joi.boolean(),
   url:         Joi.string().required(),
-  query:       Joi.any(),
+  query:       Joi.object(),
 })
 
 module.exports = composeP(gimme, validate(schema))

lib/gimme.js

@@ -1,3 +1,4 @@
+const chalk = require('chalk')
 const http  = require('http')
 const https = require('https')
 const qs    = require('qs')
@@ -9,6 +10,7 @@ const {
   merge, nthArg, pick, pipe, replace, tryCatch
 } = require('ramda')
 
+const { name }   = require('../package')
 const parseBody  = require('./parseBody')
 const streamBody = require('./streamBody')
 const wrapError  = require('./wrapError')
@@ -23,29 +25,36 @@ const clean = pipe(
 const parseJSON = tryCatch(JSON.parse, nthArg(1))
 
 const gimme = opts => {
-  const { json = true } = opts
-
   const {
     data,
+    json = true
+  } = opts
+
+  if (data) console.warn(chalk.yellow(`[${name}] The 'data' option is deprecated in favor of 'body'.`))
+
+  const {
+    body        = data,
     deserialize = json ? parseJSON : identity,
     jwt,
     method      = 'GET',
     serialize   = json ? JSON.stringify : identity,
     stream      = false,
-    url,
-    query,
   } = opts
 
   const headers = merge({}, opts.headers)
 
-  const { auth, hostname, pathname, port, protocol } = URL.parse(url)
-
   if (json) headers['content-type']  = 'application/json'
   if (jwt)  headers['authorization'] = `Bearer ${jwt}`
 
-  const search = (query == null || Object.keys(query).length === 0)
-    ? ''
-    : `?${qs.stringify(query)}`
+  const parsed = URL.parse(opts.url)
+
+  const { auth, hostname, pathname, port, protocol } = parsed
+
+  const query = merge(qs.parse(parsed.query), opts.query)
+
+  const search = Object.keys(query).length > 0
+    ? `?${qs.stringify(query)}`
+    : ''
 
   const path = pathname + search
 
@@ -56,8 +65,10 @@ const gimme = opts => {
       res.on('error', reject)
 
       if (res.statusCode >= 400) {
+        const url = URL.format({ auth, hostname, pathname, port, protocol, search })
+
         const context = {
-          req: { data, headers: redact(headers), method, url },
+          req: { body, headers: redact(headers), method, url },
           res: clean(res)
         }
 
@@ -75,15 +86,16 @@ const gimme = opts => {
       }
     }
 
-    const req = (protocol === 'https:' ? https : http).request(params, respond)
+    const requester = protocol === 'https:' ? https : http
+    const req = requester.request(params, respond)
 
     req.on('error', reject)
 
-    if (method !== 'GET' && data) {
-      if (is(Readable, data)) {
-        data.pipe(req)
+    if (method !== 'GET' && body) {
+      if (is(Readable, body)) {
+        body.pipe(req)
       } else {
-        req.end(serialize(data))
+        req.end(serialize(body))
       }
     } else {
       req.end()

lib/wrapError.js

@@ -2,7 +2,7 @@ const Boom = require('boom')
 
 const wrapError = context => {
   const { statusCode, statusMessage } = context.res
-  return Boom.create(statusCode, statusMessage, context)
+  return new Boom(statusMessage, { data: context, statusCode })
 }
 
 module.exports = wrapError

package.json

@@ -23,15 +23,17 @@
     "test:coverage": "nyc yarn test"
   },
   "dependencies": {
-    "@articulate/funky": "^0.0.2",
-    "boom": "^5.2.0",
-    "joi": "^10.6.0",
-    "media-typer": "^0.3.0",
-    "qs": "^6.5.0",
-    "ramda": "^0.24.1",
-    "raw-body": "^2.2.0"
+    "@articulate/funky": "^1.6.0",
+    "boom": "^7.3.0",
+    "chalk": "^2.4.2",
+    "joi": "^14.3.1",
+    "media-typer": "^1.0.1",
+    "qs": "^6.7.0",
+    "ramda": "^0.26.1",
+    "raw-body": "^2.3.3"
   },
   "devDependencies": {
+    "@articulate/spy": "^0.0.1",
     "chai": "^4.1.1",
     "coveralls": "^2.13.1",
     "eslint": "^4.3.0",

test/00-setup.js

@@ -1,5 +1,6 @@
 const nock = require('nock')
 const qs   = require('qs')
+const spy  = require('@articulate/spy')
 const URL  = require('url')
 
 const { curry, compose, prop } = require('ramda')
@@ -28,6 +29,7 @@ const respond = curry(function(method, uri, body) {
   return [ 200, resBody, headers ]
 })
 
+console.warn = spy()
 nock.disableNetConnect()
 
 beforeEach(() => {

test/body.js

@@ -0,0 +1,47 @@
+const { expect } = require('chai')
+const property   = require('prop-factory')
+const stream     = require('stream')
+
+const gimme   = require('..')
+const { url } = require('./00-setup')
+
+describe('body', () => {
+  const body = { foo: 'bar' }
+  const res  = property()
+
+  beforeEach(() => {
+    res(undefined)
+  })
+
+  describe('when body is not a stream', () => {
+    beforeEach(() =>
+      gimme({ body, method: 'POST', url }).then(res)
+    )
+
+    it('serializes it in the request body', () =>
+      expect(res().body.body).to.eql(body)
+    )
+  })
+
+  describe('when body is a stream', () => {
+    const body  = new stream.PassThrough()
+    const value = 'some streamed body'
+
+    beforeEach(() => {
+      const promise = gimme({
+        body,
+        deserialize: JSON.parse,
+        json: false,
+        method: 'POST',
+        url
+      }).then(res)
+
+      body.end(value)
+      return promise
+    })
+
+    it('pipes the body into the request', () =>
+      expect(res().body.body).to.equal(value)
+    )
+  })
+})

test/data.js

@@ -1,49 +1,26 @@
 const { expect } = require('chai')
 const property   = require('prop-factory')
-const stream     = require('stream')
 
 const gimme   = require('..')
 const { url } = require('./00-setup')
 
 describe('data', () => {
-  const data = { foo: 'bar' }
-  const res  = property()
+  const body  = { baz: 'bop' }
+  const res   = property()
 
   beforeEach(() => {
     res(undefined)
   })
 
-  describe('when method is not GET', () => {
-    describe('and data is not a stream', () => {
-      beforeEach(() =>
-        gimme({ data, method: 'POST', url }).then(res)
-      )
+  beforeEach(() =>
+    gimme({ data: body, method: 'POST', url }).then(res)
+  )
 
-      it('serializes it in the request body', () =>
-        expect(res().body.body).to.eql(data)
-      )
-    })
+  it('is used for non-GET request body', () =>
+    expect(res().body.body).to.eql(body)
+  )
 
-    describe('and data is a stream', () => {
-      const data  = new stream.PassThrough()
-      const value = 'some streamed data'
-
-      beforeEach(() => {
-        const promise = gimme({
-          data,
-          deserialize: JSON.parse,
-          json: false,
-          method: 'POST',
-          url
-        }).then(res)
-
-        data.end(value)
-        return promise
-      })
-
-      it('pipes the data into the request', () =>
-        expect(res().body.body).to.equal(value)
-      )
-    })
-  })
+  it('is deprecated', () =>
+    expect(console.warn.calls[0][0]).to.include('deprecated')
+  )
 })

test/json.js

@@ -33,14 +33,14 @@ describe('json', () => {
   })
 
   describe('when false', () => {
-    const data = 'just a string'
+    const body = 'just a string'
 
     beforeEach(() =>
-      gimme({ data, json: false, method: 'POST', url }).then(res)
+      gimme({ body, json: false, method: 'POST', url }).then(res)
     )
 
     it('does not serialize as JSON', () =>
-      expect(JSON.parse(res().body).body).to.equal(data)
+      expect(JSON.parse(res().body).body).to.equal(body)
     )
 
     it('does not deserialize as JSON', () =>
@@ -53,14 +53,14 @@ describe('json', () => {
   })
 
   describe('when true', () => {
-    const data = { foo: 'bar' }
+    const body = { foo: 'bar' }
 
     beforeEach(() =>
-      gimme({ data, json: true, method: 'POST', url }).then(res)
+      gimme({ body, json: true, method: 'POST', url }).then(res)
     )
 
     it('serializes as JSON', () =>
-      expect(res().body.body).to.eql(data)
+      expect(res().body.body).to.eql(body)
     )
 
     it('deserializes as JSON', () =>