Skip to content

feat(core): middleware runs once for matching route#9809

Merged
kamilmysliwiec merged 3 commits intonestjs:10.0.0from
rychardvale:fix-middleware-multiple-runs
Jun 12, 2023
Merged

feat(core): middleware runs once for matching route#9809
kamilmysliwiec merged 3 commits intonestjs:10.0.0from
rychardvale:fix-middleware-multiple-runs

Conversation

@rychardvale
Copy link
Copy Markdown
Contributor

@rychardvale rychardvale commented Jun 20, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

Currently, routes that has dynamic params can trigger a middleware multiple times for a single request if matches another static route inside the controller.

Issue Number: #1628

What is the new behavior?

Routes will only trigger the middleware for the exact path match.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@coveralls
Copy link
Copy Markdown

coveralls commented Jun 20, 2022

Pull Request Test Coverage Report for Build c179baca-5af3-4693-b615-af5c35fafab1

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 0.0%

Totals Coverage Status
Change from base Build a1d27e3b-ce8d-4a6c-bebb-92a0cc67fc15: 0.0%
Covered Lines: 0
Relevant Lines: 0

💛 - Coveralls

@rychardvale rychardvale force-pushed the fix-middleware-multiple-runs branch from 9f50226 to 3281696 Compare June 20, 2022 10:30
Comment thread packages/core/middleware/builder.ts Outdated
Comment thread packages/core/middleware/builder.ts Outdated
Copy link
Copy Markdown
Contributor

@Tony133 Tony133 left a comment

Choose a reason for hiding this comment

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

A little convention/syntax fix while otherwise fine, but let's see what Kamil thinks.

Comment thread packages/core/middleware/builder.ts Outdated
Co-authored-by: Antonio T. as Tony <Tony133@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants