express icon indicating copy to clipboard operation
express copied to clipboard

Mounted Routes with JS Classes

Open jared-leddy opened this issue 2 years ago • 5 comments

The goal is to use the JS Class version of code, which is more efficient long term. When using the CLASS based JS, in the app.js file sample below, an error is received.

// USERROUTER.JS FILE, MODULE.EXPORTS EXAMPLE
const router = express.Router();

router.post('/register', authController.signup);
router.post('/login', authController.login);
router.get('/logout', authController.logout);

module.exports = router;
// USERROUTER.JS FILE, CLASS EXAMPLE
export class UserRouter {
    constructor() {}
    // Open to all users
    register() {
        router.post('/account-register', AuthController.prototype.register);
    }
    login() {
        router.post('/account-login', AuthController.prototype.login);
    }
    logout() {
        router.get('/account-logout', AuthController.prototype.logout);
    }
}
// APP.JS FILE
// Mounted API Routes
const apiVersion = '1.0';
const apiRoutes = `/api/v${apiVersion}`;
app.use(`${apiRoutes}/users`, UserRouter);

NOTE
This project is using Typescript.

Class Example Error: TypeError: Class constructor UserRouter cannot be invoked without 'new'

When using "new" keyword, a different error is received.

// APP.JS FILE, Using the "new" keyword
app.use(`${apiRoutes}/users`, new UserRouter() );

Error W/ New Keyword: TypeError: Router.use() requires a middleware function but got a Object

jared-leddy avatar May 17 '22 12:05 jared-leddy

[...] which is more efficient long term

I'd be careful with this idea; while I'm of belief efficiency comes down to the team and what's therefore best for the team, the community is much more strongly opinionated (the Node.js community that is).

Now regarding your issue, that TypeError actually comes from the router package, located:

  • https://github.com/pillarjs/router/blob/master/lib/route.js#L215

A few approaches are available; I elected to use reflection (see below).

As a disclaimer, and after giving this method a shot myself, I strongly advise against using classes.

const express = require("express");

const app = express();

class UserRouter {
    constructor() {
        this.router = express.Router();
        
        this.routes = [
            this.base
        ];
    }

    static methods = (instance) => {
        const state = {
            instance, properties: new Set()
        };

        do {
            Object.getOwnPropertyNames(state.instance).map(item => state.properties.add(item))
        } while ((state.instance = Object.getPrototypeOf(state.instance)));

        return [...state.properties.keys()].filter(item => typeof instance[item] === 'function')
    }

    static initialize = () => {
        const instance = new UserRouter();

        instance.routes.forEach(($) => {
            Reflect.set($, "router", true);
        });

        const methods = UserRouter.methods(instance);

        methods.forEach(($) => {
            const callable = instance[$];
            if (callable["router"] === true) {
                callable(instance.router);
            }
        });

        return instance.router;
    }

    base (router = this?.router) {
        router.get("/", (req, res) => {
            res.send({
                message: "Hello world"
            })
        });
    }
}

app.use("/", UserRouter.initialize());
app.listen(3000, "localhost", () => {
    console.log("Listening on http://localhost:3000");
});

Segmentational avatar May 17 '22 17:05 Segmentational

Lastly, as I recommended against using classes, this is because there needs to be a running list, per instance, of applicable functions to call. This is due to the router itself not updating until a method ("get", "put", etc. etc) is explicitly called.

I tried my best to ease those efforts, but reflection was the only means I thought of (which requires that list in the first place).

Segmentational avatar May 17 '22 18:05 Segmentational

I'd be careful with this idea...

Thanks for the reply. The idea was to use classes for OOP. With the solution example provided, this seems quite extensive for a "seemingly simple" solution. Starting to sound like Express, Typescript and classes are not the way to go.

I'd be careful with this idea...

Lastly, as I recommended against using classes...

With these in mind, I just might have to abandon classes. Which may also take me down a path to abandon Typescript too. I'm not clear on where the "running list" of functions is at.

jared-leddy avatar May 17 '22 18:05 jared-leddy

Why not return a router and 'attach' the class instance to your app?

For example, you could do something like this:

// USERROUTER.JS FILE, CLASS EXAMPLE
export class UserRouter {
    constructor() {
        this.authController = new AuthController() // this should probably also go inside attach() to access app context
    }

    attach(app) {
        // app is your global app, useful as a context to retrieve settings with app.get()
        return express.Router()
            .post('/register', this.register.bind(this))
            .post('/login', this.login.bind(this))
            .get('/logout', this.logout.bind(this))
    }

    register(req, res, next) {
        return this.authController.register(req, res, next)
    }

    login(req, res, next) {
        return this.authController.login(req, res, next)
    }

    logout(req, res, next) {
        return this.authController.logout(req, res, next)
    }

}

Then, attach the router class:

// APP.JS FILE
// Mounted API Routes
const apiVersion = '1.0';
const apiRoutes = `/api/v${apiVersion}`;
app.use(`${apiRoutes}/users`, new UserRouter.attach(app));

silentjohnny avatar May 17 '22 18:05 silentjohnny

Why not return a router and 'attach' the class instance to your app?

@silentjohnny interesting concept. Not exactly what I'm looking for, but it might work. Will give this a try and see how it goes.

jared-leddy avatar May 17 '22 18:05 jared-leddy

this is my blog routes

export default class BlogRoutes {
  constructor() {
    this.blogController = new BlogController();
    this.authenticationMiddleware = new AuthenticationMiddleware();
    this.deviceDetector = new DeviceDetectorMiddleware();
  }

  /* initialize blog routes */
  attach = (app) =>
    express
      .Router()
      .post(
        '/',
        this.deviceDetector.isDeviceBot,
        this.authenticationMiddleware.verifyApiKey,
        this.addBlog.bind(this)
      )
      .get(
        '/blog/:id',
        this.authenticationMiddleware.verifyApiKey,
        this.searchBlogById.bind(this)
      )
      .get(
        '/',
        this.authenticationMiddleware.verifyApiKey,
        this.searchAllBlogs.bind(this)
      )
      .patch(
        '/blog/:id',
        this.authenticationMiddleware.verifyApiKey,
        this.updateBlog.bind(this)
      )
      .delete(
        '/blog/:id',
        this.authenticationMiddleware.verifyApiKey,
        this.deleteBlog.bind(this)
      );

  addBlog(req, res, next) {
    return this.blogController.addBlog(req, res, next);
  }

  searchBlogById(req, res, next) {
    return this.blogController.searchBlogById(req, res, next);
  }

  searchAllBlogs(req, res, next) {
    return this.blogController.searchAllBlogs(req, res, next);
  }

  updateBlog(req, res, next) {
    return this.blogController.updateBlog(req, res, next);
  }

  deleteBlog(req, res, next) {
    return this.blogController.deleteBlog(req, res, next);
  }
}```

this is my app js
```javascript
app.use('/api/v1/blogs', new BlogRoutes.attach(app));

but am getting an error

TypeError: BlogRoutes.attach is not a constructor
    at file:///home/moobi-kabelo/Github/Afridek/beempowered/backend/app.js:44:26
    at ModuleJob.run (node:internal/modules/esm/module_job:198:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:409:24)
    at async loadESM (node:internal/process/esm_loader:85:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

the-berufegoru avatar Nov 27 '22 19:11 the-berufegoru

I moved on to Nest.js. It's basically a TypeScript bolt-on for Express. Closing ticket.

jared-leddy avatar Nov 27 '22 19:11 jared-leddy

@d3m0n-533d maybe I’m wrong, but the correct syntax should be:

new BlogRoutes().attach(app)

see you Romain

Romakita avatar Nov 27 '22 22:11 Romakita