8000 fix(release): sort groups topologically bottom-up and fix typo to allow multi-level group dependencies by mpsanchis · Pull Request #31374 · nrwl/nx · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix(release): sort groups topologically bottom-up and fix typo to allow multi-level group dependencies #31374

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

Merged
merged 1 commit into from
Jun 6, 2025

Conversation

mpsanchis
Copy link
Contributor
@mpsanchis mpsanchis commented May 28, 2025

Current Behavior

  • topological sorting returns array top-bottom (i.e.: if A -> B -> C, it returns [A, B, C])
  • multi-level dependencies are not bumped (i.e.: if A -> B -> C and C is bumped, B is bumped but A is not)
  • a set: Set<string> is accessed with set[0], which always returns undefined

Expected Behavior

  • topological sorting returns array bottom-up ([C, B, A]), because we typically want to deal with leaves first, and then "touch" the intermediate nodes as we travel up the tree
  • multi-level dependencies are bumped (C bumps B, which bumps A)
  • set is accessed with set.values().next().value, assuming we want to get any one element from it

Related Issue(s)

Fixes #31375

8000
@mpsanchis mpsanchis requested a review from a team as a code owner May 28, 2025 12:15
@mpsanchis mpsanchis requested a review from FrozenPandaz May 28, 2025 12:15
Copy link
vercel bot commented May 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview Jun 5, 2025 2:42pm

Copy link
nx-cloud bot commented May 28, 2025

View your CI Pipeline Execution ↗ for commit d34d3c9.

Command Status Duration Result
nx affected --targets=lint,test,build,e2e,e2e-c... ✅ Succeeded 42m 31s View ↗
nx run-many -t check-imports check-commit check... ✅ Succeeded 16s View ↗
nx-cloud record -- nx format:check ✅ Succeeded 4s View ↗
nx-cloud record -- nx sync:check ✅ Succeeded 2s View ↗
nx documentation ✅ Succeeded 1m 3s View ↗

☁️ Nx Cloud last updated this comment at 2025-06-05 15:16:24 UTC

@JamesHenry
Copy link
Collaborator

Thanks a lot for this @mpsanchis! You'll need to please update some e2e snapshots as a result of this change. We capture the user experience as best as we can via the snapshots so with the ordering of things changes it impacts those

Copy link
Collaborator
@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

See comment

@JamesHenry JamesHenry removed the request for review from FrozenPandaz May 28, 2025 15:15
@JamesHenry JamesHenry self-assigned this May 28, 2025
@mpsanchis
Copy link
Contributor Author

@JamesHenry I adapted the e2e :)
Let me know if you want more changes!

Copy link
Collaborator
@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

Thank you!

@JamesHenry JamesHenry merged commit fa9290a into nrwl:master Jun 6, 2025
8 checks passed
@mpsanchis mpsanchis deleted the feat/fix-release-dependencies branch June 6, 2025 11:25
FrozenPandaz pushed a commit that referenced this pull request Jun 6, 2025
…ow multi-level group dependencies (#31374)

(cherry picked from commit fa9290a)
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nx release does not bump across multiple group boundaries
2 participants
0