Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

defaults can only be called once #607

Closed
daleharvey opened this Issue · 11 comments

5 participants

@daleharvey
request = request.defaults({json: true});
request = request.defaults({json: false});

is a silly example, but gives me

request = request.defaults({json: false});
                  ^
TypeError: Object function (uri, opts, callback) {
      var params = initParams(uri, opts, callback)
      for (var i in options) {
        if (params.options[i] === undefined) params.options[i] = options[i]
      }
      if(typeof requester === 'function') {
        if(method === request) {
          method = requester
        } else {
          params.options._requester = requester
        }
      }
      return method(params.options, params.callback)
    } has no method 'defaults'
    at Object.<anonymous> (/Users/daleharvey/src/persona-couch-provision/persona-couch-provision.js:35:19)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:901:3

Not a big deal since its easy to workaround

@jasonkuhrt

I would like to see this fixed. In testing environments nested defaults is great.

For instance I want a global defaults for setting a ca and then each API endpoint describe block gets another layer of defaults so that the it tests can share a common request template.

@jasonkuhrt

https://github.com/mikeal/request/blob/master/index.js#L1287

A patch (using lodash, say) like this seems to work locally in my project (see new de.defaults line below):

...
  var de = def(request)
  de.get = def(request.get)
  de.patch = def(request.patch)
  de.post = def(request.post)
  de.put = def(request.put)
  de.head = def(request.head)
  de.del = def(request.del)
  de.cookie = def(request.cookie)
  de.jar = request.jar
  de.defaults = function(moreOptions, requester){
    return request.defaults(_.merge({}, options, moreOptions), requester)
  }
  return de
}
...

@mikeal this seems to be an easy fix and the feature request seems sound. I do not know how you'd like to solve it though (namely how you'd want to perform the deep merge). Any word on this?

@tikotzky

#890 adds the ability for you to call defaults on the request that is returned from defaults

@mikeal
Owner

this is fixed.

@mikeal mikeal closed this
@nylen
Owner

Not fixed; waiting on tests and docs in #890.

@mikeal
Owner

I'm pretty sure a PR was merged for recursive defaults, was it not merged?

@mikeal mikeal reopened this
@nylen
Owner

Doesn't look like it - the only one I've seen come through is #890, and if it were merged, https://github.com/mikeal/request/blob/master/index.js#L92 would say var de = def(this).

@mikeal
Owner

damn, ok, adding proper labels.

@tikotzky

I'll try to work on adding tests to #890

@tikotzky

@nylen @mikeal just opened #1030 that adds support for recursive defaults with docs and tests

@nylen
Owner

Fixed at #1030.

@nylen nylen closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.