Skip to content

Commit

Permalink
feat: Remove the restore call (#9)
Browse files Browse the repository at this point in the history
* feat: Remove the restore call

* remove commented code
  • Loading branch information
joeyguerra authored Jan 20, 2024
1 parent 6108ce3 commit 50cd641
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 46 deletions.
1 change: 1 addition & 0 deletions History.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
===

* deps: Remove setprototypeof
* no longer need to call the restore method to restore `req.params` after invoking router

5.x
===
Expand Down
17 changes: 1 addition & 16 deletions lib/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,22 +197,7 @@ class ExpressApp extends EventEmitter {
#mergeMethodsFromTargetToSource(source, target) {
Object.keys(source).forEach(key => target[key] = source[key].bind(target))
}
#mergeRequestAndResponse (req, res, fn) {
if (req.originalUrl === req.baseUrl) {
this.#mergeMethodsFromTargetToSource(fn.request, req)
this.#mergeMethodsFromTargetToSource(fn.response, res)
} else {
this.#mergeMethodsFromTargetToSource(this.request, req)
this.#mergeMethodsFromTargetToSource(this.response, res)
}
}
#wrapper (fn) {
return (req, res, next) => {
this.#mergeMethodsFromTargetToSource(this.request, req)
this.#mergeMethodsFromTargetToSource(this.response, res)
fn(req, res, next)
}
}

/**
* Proxy `Router#use()` to add middleware to the app router.
* See Router#use() documentation for details.
Expand Down
27 changes: 1 addition & 26 deletions lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ Router.prototype.handle = function handle (req, res, callback) {
// manage inter-router variables
var parentParams = req.params
var parentUrl = req.baseUrl || ''
var done = restore(callback, req, 'baseUrl', 'next', 'params')
var done = callback

// setup next layer
req.next = next
Expand Down Expand Up @@ -635,31 +635,6 @@ function processParams (params, layer, called, req, res, done) {
param()
}

/**
* Restore obj props after function
*
* @private
*/

function restore (fn, obj) {
var props = new Array(arguments.length - 2)
var vals = new Array(arguments.length - 2)

for (var i = 0; i < props.length; i++) {
props[i] = arguments[i + 2]
vals[i] = obj[props[i]]
}

return function () {
// restore vals
for (var i = 0; i < props.length; i++) {
obj[props[i]] = vals[i]
}

return fn.apply(this, arguments)
}
}

/**
* Send an OPTIONS response.
*
Expand Down
10 changes: 6 additions & 4 deletions test/AppRouter.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ describe('app.router', () => {
it('should restore req.params after leaving router', (t, done) => {
const app = express()
const router = new express.Router()
const server = app.listen()

function handler1(req, res, next){
res.setHeader('x-user-id', String(req.params.id))
Expand All @@ -30,11 +31,11 @@ describe('app.router', () => {

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

request(app)
request(server)
.get('/user/1')
.expect('x-router', 'undefined')
.expect('x-user-id', '1')
.expect(200, '1', done)
.expect(200, '1', () => server.close(done))
})

describe('methods', () => {
Expand Down Expand Up @@ -382,6 +383,7 @@ describe('app.router', () => {

it('should restore req.params', (t, done) => {
const app = express()
const server = app.listen()
const router = new express.Router({ mergeParams: true })

router.get('/user-(\\w+)/(.*)', (req, res, next) => {
Expand All @@ -395,9 +397,9 @@ describe('app.router', () => {
})
})

request(app)
request(server)
.get('/user/id-42/user-tj/profile')
.expect(200, '[["0","42"]]', done)
.expect(200, '[["0","42"]]', () => server.close(done))
})
})

Expand Down

0 comments on commit 50cd641

Please sign in to comment.