BackendStack21 / restana

Restana is a lightweight and fast Node.js framework for building RESTful APIs.
MIT License
467 stars 27 forks source link

Accept boolean in res.send #98

Closed schamberg97 closed 4 years ago

schamberg97 commented 4 years ago

Ok, I am on a roll :D Sorry for this :D

Code

service.get('/hi', (req, res) => {
  res.send(true)
})

Response

{
"code":500,
"message":"The \"chunk\" argument must be of type string or an instance of Buffer. Received type boolean (true)"
}

Solution

Stringify if type of data is boolean

jkyberneees commented 4 years ago

Hi @schamberg97, thanks again!

However, in this case I don't see a real need for this feature. This is not even support by express: https://expressjs.com/en/api.html#res.send or the Node.js HTTP API. I must say, there is an intentional (performance) reason to keep the send method API as strict as possible. In this particular case I would prefer to reject this PR, until there is a strong argument to otherwise.

Regards

schamberg97 commented 4 years ago

Technically, they do support Boolean, they just failed to address it in their Documentation :D

Here's a part of their code for res.send:


switch (typeof chunk) {
    // string defaulting to html
    case 'string':
      if (!this.get('Content-Type')) {
        this.type('html');
      }
      break;
    case 'boolean':   // <--- this
    case 'number':
    case 'object':
      if (chunk === null) {
        chunk = '';
      } else if (Buffer.isBuffer(chunk)) {
        if (!this.get('Content-Type')) {
          this.type('bin');
        }
      } else {
        return this.json(chunk);   // <--- and this
      }
      break;
  }

Even their tests depend on this undocumented feature:

it('should return true when the resource is not modified', function(done){
      var app = express();
      var etag = '"12345"';

      app.use(function(req, res){
        res.set('ETag', etag);
        res.send(req.fresh); // <-- req.fresh returns Boolean
      });

      request(app)
      .get('/')
      .set('If-None-Match', etag)
      .expect(304, done);
    })

But, having said that, I see you point and concur, it will unnecessarily decrease performance. Those who need it will be clever enough to make their middleware :D

jkyberneees commented 4 years ago

Thanks!