vkarpov15 / awaitjs-express

Write Express middleware and route handlers using async/await
Apache License 2.0
125 stars 13 forks source link

Use `decorateApp` to decorate a `Router`? #1

Closed jloveridge closed 5 years ago

jloveridge commented 6 years ago

Wanted to know if decorateApp can/should be used on a Router instance as well?

// In `app.js`
const express = require('express');
const { decorateApp } = require('@awaitjs/express');
const app = decorateApp(express());

// in some route file
const express = require('express');
const { decorateApp } = require('@awaitjs/express');
const router = decorateApp(express.Router());

router.getAsync('oops', async (req, res, next) => {
    throw new Error('Oops!');
});
jloveridge commented 6 years ago

Looking at the code my assumption is yes, decorateApp can be used on app or router instances interchangeably.

vkarpov15 commented 6 years ago

Yes it can. We need to add that to the docs. Maybe add decorateRouter() as an alias to avoid confusion?

loris commented 6 years ago

Or just using const app = decorateApp(express()); should also decorate any router instances created from this app? ie

// In `app.js`
const express = require('express');
const { decorateApp } = require('@awaitjs/express');
const app = decorateApp(express());

// in some route file
const express = require('express');
const router = express.Router();

router.getAsync('oops', async (req, res, next) => {
    throw new Error('Oops!');
});
jloveridge commented 6 years ago

The issue with @loris suggestion is that decorateApp doesn't do anything to express it adds methods to the object returned by calling express(). I think @vkarpov15 suggestion of adding decorateRouter as an alias would work fine. Alternatively I would just rename decorateApp to something like makeAsync/addAsync and make decorateApp an alias to that method to avoid a breaking change.