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(core): revisit dependencies w/ possibly higher distance #14097

Merged
merged 1 commit into from
Nov 8, 2024

Conversation

clkamp
Copy link

@clkamp clkamp commented Oct 29, 2024

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?

Issue Number: #14096

What is the new behavior?

The module distance is reevaluated when the module is not within the modules visited in the current depth search. This should be enough to avoid endless loops on cyclic dependency graphs. However, modules that have dependency paths of different length are now evaluated to the higher distance.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@coveralls
Copy link

Pull Request Test Coverage Report for Build c981fc60-fb24-410d-bf21-d0e1e65b9f9b

Details

  • 4 of 5 (80.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.243%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/core/scanner.ts 4 5 80.0%
Totals Coverage Status
Change from base Build 3047ae8a-a5a4-402c-9e7d-49c14e1ba997: 0.0%
Covered Lines: 6754
Relevant Lines: 7322

💛 - Coveralls

@arabold
Copy link

arabold commented Oct 30, 2024

Just ran into this today as well and can confirm that the fixes my issue. Now my database service is properly initialized before being used in other modules. 👍

Copy link

@Vue-Pu Vue-Pu left a comment

Choose a reason for hiding this comment

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

Closes #14097

@Vue-Pu
Copy link

Vue-Pu commented Nov 3, 2024

@arabold @clkamp review needed to closePR no breaking changes will accur. and fixes #14097

@kamilmysliwiec kamilmysliwiec changed the base branch from master to 11.0.0 November 8, 2024 10:42
@kamilmysliwiec kamilmysliwiec added this to the 11.0.0 milestone Nov 8, 2024
@kamilmysliwiec kamilmysliwiec merged commit 84bf570 into nestjs:11.0.0 Nov 8, 2024
3 checks passed
@kamilmysliwiec kamilmysliwiec mentioned this pull request Nov 20, 2024
12 tasks
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