Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

req.params.all() returns TRUE #2524

Closed
rumyhuampas opened this issue Dec 31, 2014 · 9 comments
Closed

req.params.all() returns TRUE #2524

rumyhuampas opened this issue Dec 31, 2014 · 9 comments
Assignees

Comments

@rumyhuampas
Copy link

Hi.
We are having issues with sails when parsing req values in blueprints actions.
req.params.all() instead of returning the same as req.allParams() it returns TRUE.
Why could this be happening? They are suppossed to be synonyms right?
req.allParams() works fine and returns an object with all request params.

Node 0.10.33
Sails 0.10.5

Any help is appreciated.

@wyqydsyq
Copy link

wyqydsyq commented Jan 7, 2015

I'm running node 0.11.14 with sails 0.10.5 and req.params.all() returns an array/object of the parameters as expected. Try upgrading your node?

@rumyhuampas
Copy link
Author

I have upgraded to node 0.11.14 and req.params.all() still returns true!
Is really weird...

This is my params.all file in sails/lib/hooks/request/

/**
 * Module dependencies
 */
var _ = require('lodash');
var defaultsDeep = require('merge-defaults');



/**
 * _mixinReqParamsAll
 *
 * Mixes in `req.params.all()`, a convenience method to grab all parameters,
 * whether they're in the path (req.params), query string (req.query),
 * or request body (req.body).
 *
 * Note: this has to be applied per-route, not per request,
 * in order to refer to the proper route/path parameters
 *
 * @param {Request} req
 * @param {Response} res
 * @api private
 */

module.exports = function _mixinReqParamsAll(req, res) {

  ////////////////////////////////////////////////////////////////////
  // The check below is deprecatable since action blueprints
  // will no longer automatically receive the :id? param in their route.
  //
  // // Make sure `id` is omitted if it's undefined
  // // (since action blueprint routes name an optional :id)
  // if (typeof req.param('id') === 'undefined') {
  //    delete req.params.id;
  // }
  //////////////////////////////////////////////////////////////////

  // Combines parameters from the query string, and encoded request body
  // to compose a monolithic object of named parameters, irrespective of source
  var queryParams = _.cloneDeep(req.query) || {};
  var bodyParams = _.cloneDeep(req.body) || {};
  var allParams = {};
  defaultsDeep(allParams, queryParams);
  defaultsDeep(allParams, bodyParams);


  // Mixin route params
  _.each(Object.keys(req.params), function(paramName) {
    allParams[paramName] = allParams[paramName] || req.params[paramName];
  });

  // Define a new non-enuerable property: `req.allParams()`
  req.allParams = function () {
    return allParams;
  };


  // Define a new non-enuerable property: req.params.all()
  // and make it a synonym to `req.allParams()`
  // (but only if `req.params.all` doesn't already exist!)
  if (!req.params.all) {
    Object.defineProperty(req.params, 'all', {
      value: function getAllParams() {
        return allParams;
      }
    });
  }

};

What are we doing wrong?

Please help!!!

@CWyrtzen
Copy link

Are you still having issues after v0.11?

@ejohnsonw
Copy link

had this problem just yesterday. when changed to req.allParams() it works as intended. I'm using 0.11.0

@ejohnsonw
Copy link

problem appears to be on the params.all.js. Disabling the if alleviates the problem.

if (!req.params.all) {
Object.defineProperty(req.params, 'all', {
value: function getAllParams() {
return allParams;
}
});
}

@billyshena
Copy link

Hello guys, I'm currently having the same issue with node 5.0.0 and sails 0.11.2.
My blueprints action like GET /pets?name=toto does not work anymore if I append a condition (?field=...).
I have tried to debug and req.params.all() also returns TRUE instead of the where clause.
Any ideas?

Best,

@Zaggen
Copy link

Zaggen commented Jan 11, 2016

Same here, using node 4.2.4 and sails 0.11.3, switching to req.allParams() works though...

@Bazze
Copy link

Bazze commented Oct 17, 2016

I just encountered this very same issue. Using sails 0.12.7 and node 4.3.0. It happens on my feature branch but not on master, so I'm gonna try to find the culprit!

@Bazze
Copy link

Bazze commented Oct 17, 2016

Okay, I've debugged for hours and finally found the issue. So I went into the params.all.js file and logged the value of req.params.all just before the if, it showed me the following value:

// console.log(req.params.all)
function () {
    return this.every(Boolean);
}

Very interesting. I decided to do a grep -R "this.every(Boolean)" node_modules in my project which resulted in one match in a package called collections. I scrolled down in the file and found an interesting require statement which got my attention: "shim-array" (!?). That didn't sound good.

In the shim-array.js file I found the culprit. They actually define properties, all amongst others, on the Array prototype in the global scope, effectively destroying the req.params.all() feature in sails. I found an issue regarding this in their issue tracker (montagejs/collections#162) where they seem to solve the issue by downgrading to collections@2.0.2. This solution seems to work for me too, but since the collections package is a dependency of my dependency I need to make sure they are compatible.

Bazze pushed a commit to Bazze/memcache-plus that referenced this issue Oct 17, 2016
The "collections" package has issues with array shims causing other
dependencies to break when least expected. This has been discussed
in multiple issues:

 - montagejs/collections#162
 - montagejs/collections#95
 - balderdashy/sails#2524

Another solution to this could be to depend on "collections@2.0.2"
instead which is a version without shims.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

8 participants