Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: middleware sort issues #14523

Merged
merged 2 commits into from
Jan 28, 2025
Merged

fix: middleware sort issues #14523

merged 2 commits into from
Jan 28, 2025

Conversation

rbnayax
Copy link
Contributor

@rbnayax rbnayax commented Jan 27, 2025

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?

Global middleware is not executed in import order, because sort is not stable

Issue Number: N/A

What is the new behavior?

Preserve order of global middleware definitions.
Sort docs state a list of rules that the sort function must adhere to. Current impl violates it and as such the order is not stable. Violation happens when 2 modules are both global but he return value is not 0. so if comparing a,b it retuns a is "bigger" and if comparing b,a it returns b is "bigger".

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

This bug was introduced in 44490dc

@coveralls
Copy link

coveralls commented Jan 27, 2025

Pull Request Test Coverage Report for Build 2b1de76b-073a-4696-be90-8ad98e874ef4

Details

  • 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 89.702%

Totals Coverage Status
Change from base Build d2a901a1-4a03-45f4-ad93-0b6f141b61fb: 0.0%
Covered Lines: 7108
Relevant Lines: 7924

💛 - Coveralls

@rbnayax
Copy link
Contributor Author

rbnayax commented Jan 27, 2025

@kamilmysliwiec can you please review this? It is a regression introduced in latest v11 patch version

@kamilmysliwiec kamilmysliwiec merged commit 65e64af into nestjs:master Jan 28, 2025
5 checks passed
@kamilmysliwiec
Copy link
Member

LGTM

@rbnayax rbnayax deleted the sort branch January 28, 2025 15:12
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.

3 participants