Skip to content

✅ add discarded middleware test#5819

Merged
UlisesGascon merged 1 commit into
masterfrom
use-mw-test
Oct 20, 2024
Merged

✅ add discarded middleware test#5819
UlisesGascon merged 1 commit into
masterfrom
use-mw-test

Conversation

@ctcpip

@ctcpip ctcpip commented Aug 9, 2024

Copy link
Copy Markdown
Member

No description provided.

Comment thread test/app.router.js
@wesleytodd

Copy link
Copy Markdown
Member

Lets not merge anything more to master until we merge 5.x. Just wanted to drop the approval here since I was going through notifications.

@UlisesGascon UlisesGascon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! but let's wait until we can merge it :)

@UlisesGascon

Copy link
Copy Markdown
Member

condition meet, I will merge the PR

@Jayx239 Jayx239 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments to better demonstrate what I want to acheive.

Comment thread test/app.router.js
.expect(200, 'yee', function(err, res) {
if (err) return done(err);

router = new express.Router();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to replace the router, I want to completely remove it.

Comment thread test/app.router.js
});

app.use(function (req, res, next) {
return router.handle(req, res, next);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To demonstrate the issue I am trying to address, replace
return router.handle(req, res, next);
with
console.log('I want to remove this'); return router.handle(req, res, next);

Is there a mechanism in this case to remove the console.log(...)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants