Skip to content

Commit

Permalink
feat: Remove the last use of setprototypeof
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Router was previously a function, but now is an object. This API changes codde like `app.use(router)` to `app.use(router.handle.bind(router)`. Checkout the `test/Router.mjs` tests.
  • Loading branch information
joeyguerra committed Jan 20, 2024
1 parent 0938aad commit 65cbd69
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 57 deletions.
7 changes: 4 additions & 3 deletions examples/multi-router/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@
var express = require('../..');

var app = module.exports = express();

app.use('/api/v1', require('./controllers/api_v1'));
app.use('/api/v2', require('./controllers/api_v2'));
const api_v1 = require('./controllers/api_v1');
const api_v2 = require('./controllers/api_v2');
app.use('/api/v1', api_v1.handle.bind(api_v1));
app.use('/api/v2', api_v2.handle.bind(api_v2));

app.get('/', function(req, res) {
res.send('Hello from root route.')
Expand Down
21 changes: 6 additions & 15 deletions lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ var methods = require('methods')
var mixin = require('utils-merge')
var parseUrl = require('parseurl')
var Route = require('./route')
var setPrototypeOf = require('setprototypeof')

/**
* Module variables.
Expand Down Expand Up @@ -52,21 +51,13 @@ function Router (options) {
}

var opts = options || {}
this.caseSensitive = opts.caseSensitive
this.mergeParams = opts.mergeParams
this.params = {}
this.strict = opts.strict
this.stack = []

function router (req, res, next) {
router.handle(req, res, next)
}

// inherit from the correct prototype
setPrototypeOf(router, this)

router.caseSensitive = opts.caseSensitive
router.mergeParams = opts.mergeParams
router.params = {}
router.strict = opts.strict
router.stack = []

return router
return this
}

/**
Expand Down
4 changes: 2 additions & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions test/AppOptions.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('OPTIONS', () => {
const router = new express.Router()

router.get('/users', (req, res) => {})
app.use(router)
app.use(router.handle.bind(router))
app.get('/other', (req, res) => {})

request(server)
Expand All @@ -90,7 +90,7 @@ describe('OPTIONS', () => {
res.writeHead(200)
next()
})
app.use(router)
app.use(router.handle.bind(router))
app.use((err, req, res, next) => {
res.end('true')
})
Expand Down
30 changes: 15 additions & 15 deletions test/AppRouter.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('app.router', () => {
next()
})

app.get('/user/:id', handler1, router, handler2)
app.get('/user/:id', handler1, router.handle.bind(router), handler2)

request(server)
.get('/user/1')
Expand Down Expand Up @@ -275,7 +275,7 @@ describe('app.router', () => {
res.send(req.params)
})

app.use('/user/:user', router)
app.use('/user/:user', router.handle.bind(router))

request(app)
.get('/user/1/get')
Expand All @@ -291,7 +291,7 @@ describe('app.router', () => {
res.send(keys.map(k => [k, req.params[k]]))
})

app.use('/user/:user', router)
app.use('/user/:user', router.handle.bind(router))

request(app)
.get('/user/tj/get')
Expand All @@ -307,7 +307,7 @@ describe('app.router', () => {
res.send(keys.map(k => [k, req.params[k]] ))
})

app.use('/user/:thing', router)
app.use('/user/:thing', router.handle.bind(router))

request(app)
.get('/user/tj/get')
Expand All @@ -323,7 +323,7 @@ describe('app.router', () => {
res.send(keys.map(k => [k, req.params[k]] ))
})

app.use('/user/id-(\\d+)', router)
app.use('/user/id-(\\d+)', router.handle.bind(router))

request(app)
.get('/user/id-10/profile.json')
Expand All @@ -339,7 +339,7 @@ describe('app.router', () => {
res.send(keys.map(k => [k, req.params[k]] ))
})

app.use('/user/id-(\\d+)/name-(\\w+)', router)
app.use('/user/id-(\\d+)/name-(\\w+)', router.handle.bind(router))

request(app)
.get('/user/id-10/name-tj/profile')
Expand All @@ -355,7 +355,7 @@ describe('app.router', () => {
res.send(keys.map(k => [k, req.params[k]] ))
})

app.use('/user/id-(\\d+)', router)
app.use('/user/id-(\\d+)', router.handle.bind(router))

request(app)
.get('/user/id-10/name-tj')
Expand All @@ -373,7 +373,7 @@ describe('app.router', () => {

app.use('/user/', (req, res, next) => {
req.params = 3; // wat?
router(req, res, next)
router.handle(req, res, next)
})

request(app)
Expand Down Expand Up @@ -870,7 +870,7 @@ describe('app.router', () => {
res.end('failure')
})

app.use(router)
app.use(router.handle.bind(router))

app.get('/foo', (req, res) => {
res.end('success')
Expand Down Expand Up @@ -958,7 +958,7 @@ describe('app.router', () => {
res.send('saw ' + err.name + ': ' + err.message)
})

app.use(router)
app.use(router.handle.bind(router))

request(app)
.get('/')
Expand All @@ -977,7 +977,7 @@ describe('app.router', () => {
res.send('saw ' + err.name + ': ' + err.message)
})

app.use(router)
app.use(router.handle.bind(router))

request(app)
.get('/')
Expand All @@ -997,7 +997,7 @@ describe('app.router', () => {
done(new Error('Unexpected middleware invoke'))
})

app.use(router)
app.use(router.handle.bind(router))

request(app)
.get('/foo')
Expand All @@ -1021,7 +1021,7 @@ describe('app.router', () => {
res.send('saw ' + err.name + ': ' + err.message)
})

app.use(router)
app.use(router.handle.bind(router))

request(app)
.get('/')
Expand All @@ -1044,7 +1044,7 @@ describe('app.router', () => {
res.send('saw ' + err.name + ': ' + err.message)
})

app.use(router)
app.use(router.handle.bind(router))

request(app)
.get('/')
Expand All @@ -1068,7 +1068,7 @@ describe('app.router', () => {
done(new Error('Unexpected middleware invoke'))
})

app.use(router)
app.use(router.handle.bind(router))

request(app)
.get('/foo')
Expand Down
14 changes: 7 additions & 7 deletions test/ReqBaseUrl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('req', () => {
sub.get('/:b', (req, res) => {
res.end(req.baseUrl)
})
app.use('/:a', sub)
app.use('/:a', sub.handle.bind(sub))

request(app)
.get('/foo/bar')
Expand All @@ -41,9 +41,9 @@ describe('req', () => {
sub3.get('/:d', (req, res) => {
res.end(req.baseUrl)
})
sub2.use('/:c', sub3)
sub1.use('/:b', sub2)
app.use('/:a', sub1)
sub2.use('/:c', sub3.handle.bind(sub3))
sub1.use('/:b', sub2.handle.bind(sub2))
app.use('/:a', sub1.handle.bind(sub1))

request(app)
.get('/foo/bar/baz/zed')
Expand All @@ -61,12 +61,12 @@ describe('req', () => {
urls.push('0@' + req.baseUrl)
next()
})
sub2.use('/:c', sub3)
sub2.use('/:c', sub3.handle.bind(sub3))
sub1.use('/', (req, res, next) => {
urls.push('1@' + req.baseUrl)
next()
})
sub1.use('/bar', sub2)
sub1.use('/bar', sub2.handle.bind(sub2))
sub1.use('/bar', (req, res, next) => {
urls.push('2@' + req.baseUrl)
next()
Expand All @@ -75,7 +75,7 @@ describe('req', () => {
urls.push('3@' + req.baseUrl)
next()
})
app.use('/:a', sub1)
app.use('/:a', sub1.handle.bind(sub1))
app.use((req, res, next) => {
urls.push('4@' + req.baseUrl)
res.end(urls.join(','))
Expand Down
2 changes: 1 addition & 1 deletion test/ResFormat.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ describe('res', () => {
res.send('Supports: ' + err.types.join(', '))
})

app.use(router)
app.use(router.handle.bind(router))

test(app)
})
Expand Down
24 changes: 12 additions & 12 deletions test/Router.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { describe, it } from 'node:test'
describe('Router', () => {
it('should return a function with router methods', () => {
const router = new Router()
assert(typeof router === 'function')
assert(router instanceof Router)

assert(typeof router.get === 'function')
assert(typeof router.handle === 'function')
Expand All @@ -22,7 +22,7 @@ describe('Router', () => {
another.get('/bar', (req, res) => {
res.end()
})
router.use('/foo', another)
router.use('/foo', another.handle.bind(another))

router.handle({ url: '/foo/bar', method: 'GET' }, { end: done}, done)
})
Expand All @@ -35,7 +35,7 @@ describe('Router', () => {
assert.strictEqual(req.params.bar, 'route')
res.end()
})
router.use('/:foo', another)
router.use('/:foo', another.handle.bind(another))

router.handle({ url: '/test/route', method: 'GET' }, { end: done}, done)
})
Expand Down Expand Up @@ -472,9 +472,9 @@ describe('Router', () => {
req.user = user
next()
})

router.use('/foo/:user/', new Router())
router.use('/foo/:user/', sub)
const r = new Router()
router.use('/foo/:user/', r.handle.bind(r))
router.use('/foo/:user/', sub.handle.bind(sub))

router.handle(req, {}, err => {
if (err) return done(err)
Expand All @@ -499,9 +499,9 @@ describe('Router', () => {
req.user = user
next()
})

router.use('/foo/:user/', new Router())
router.use('/:user/bob/', sub)
const r = new Router()
router.use('/foo/:user/', r.handle.bind(r))
router.use('/:user/bob/', sub.handle.bind(sub))

router.handle(req, {}, err => {
if (err) return done(err)
Expand Down Expand Up @@ -530,9 +530,9 @@ describe('Router', () => {
req.ms = ms
setTimeout(next, ms)
})

router.use('/foo/:ms/', new Router())
router.use('/foo/:ms/', sub)
const r = new Router()
router.use('/foo/:ms/', r.handle.bind(r))
router.use('/foo/:ms/', sub.handle.bind(sub))

router.handle(req1, {}, function(err) {
assert.ifError(err)
Expand Down

0 comments on commit 65cbd69

Please sign in to comment.