-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
updateDependents feature is not working as expected. #26041
Comments
Please provide your release config and the commands you are running |
{ have a project where I've just been through (pnpm prefixed to all my nx commands) nx release --first-release, and nx release --dry-run attempts are all clean and ready to go. I don't like the fact my changelogs get restated git history where it already exists (options for how to deal with existing changelogs maybe a feature request?) - otherwise that's ignorable. Anyways, nx release command says nothing to do, so I commit this and this is all tagged correctly etc. I edit my button component library project, and I commit this one file. I attempt to nx release again with this new awesome updateDependents I found that's actually critical to us transitioning from lerna -> nx. In the chore(release) commit, I notice it has less changelog.md files than package.json. Of the updated packages that come from this button update, I'm seeing the dependencies updated in the package.json for some packages, but their versions are't bumped, no changelog update either, and however in other projects that are updated the version of the package is correctly bumped. Another way to summarize what I'm seeing is when I perform nx release it is correctly updating all the package.jsons dependency sections, but its not updating the version on all of them, only some of expected. Therefore existing behavior before this feature was added shouldn't be affected. This is a repo with ~40ish projects. The button project seems to get processed about half way through the big loop inside the release-version.ts package, and the projects that aren't bumped are all before button is processed. The projects that are correctly bumped are all after. This is because the projectToDependencyBumps which the updateDependents feature relies on isn't populated until the original project that you updated gets processed. The fix to this might be to prioritize the originally updated package or some other mechanism - but I would also make your tests for this feature test against a bigger project graph. The graph is good in my project. |
When button gets processed, the "projectToDependencyBumps" gets filled with all the projects I expect to receive a version bump. However of course only the packages processed after button get updated/bumped and changelog correctly. |
because updateDependentProjectAndAddToVersionData is the function populating projectToDependencyBumps - the updated project(s) need to be processed first (and that too needs to ensure to update other projects already processed of course too...), or ensure that when projectToDependencyBumps is populated, projects are reprocessed. There needs to be some rearrangement of the logic of this release-version for this to work properly IMO. |
Still occuring in latest - @JamesHenry here is a screenshot of what I'm seeing. This is about half way through the process, just after it has processed button. The items in this array 0-3 have already been processed, and it is not bumping their version. It updates the button dependency version, but not their version. Because this is populated and has() is returning true for subsequent packages, those are getting updated and w/ changelog. |
@FrozenPandaz @JamesHenry any updates to this? |
Hi @rcolwell-cb thanks a lot for the write up, I'm looking into this today |
Hi again, @rcolwell-cb, thanks again for your patience on this one. The issue report contains a lot of unbroken and unformatted text and currently focuses on implementation details at a specific moment in time before a reproduction was provided. Both of these factors make it a little hard to read and understand for me, so let's please focus on a shared reproduction as a next step. I have attempted to create that for us based on the information you provided using the latest version of Nx ( Please see the repo here with the steps outlined and the If you are able to reproduce the issue, please kindly open a PR into that repo with the changes required so that we have a shared base upon which to work and I can get it resolved ASAP. Many thanks! PS I also added a screenshot of the result of running |
Can we connect somewhere for a screenshare possibly?
…________________________________
From: James Henry ***@***.***>
Sent: Sunday, June 9, 2024 7:50:52 AM
To: nrwl/nx ***@***.***>
Cc: Rhane ***@***.***>; Mention ***@***.***>
Subject: Re: [nrwl/nx] updateDependents feature is not working as expected. (Issue #26041)
Hi again, @rcolwell-cb<https://github.com/rcolwell-cb>, thanks again for your patience on this one.
The issue report contains a lot of unbroken and unformatted text and currently focuses on implementation details at a specific moment in time before a reproduction was provided. Both of these factors make it a little hard to read and understand for me, so let's please focus on a shared reproduction as a next step.
I have attempted to create that for us based on the information you provided using the latest version of Nx (19.2.2 at the time of writing). However, everything looks correct to me in the output, so either I am missing something, or the issue you observed with the canary release has since been resolved in stable versions.
Please see the repo here with the steps outlined and the nx release output it produced: https://github.com/JamesHenry/nx-26041
If you are able to reproduce the issue, please kindly open a PR into that repo with the changes required so that we have a shared base upon which to work and I can get it resolved ASAP.
Many thanks!
—
Reply to this email directly, view it on GitHub<#26041 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BBLK7NVMTNU7GFRG2DNMKLDZGQ6RZAVCNFSM6AAAAABIIGHHQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNJWGQ3DONBUG4>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
@rcolwell-cb Let's please keep things async for now, I put together the repo to use as the shared source of truth. If you want to capture you screen when analyzing something and share it then please do and I will get back to you as soon as I can |
I see your graph - I think it's going to need to be more complex to reproduce my situation The graph of this project is literally a big web. I last tested this on 19.1.1 so I'll retest on latest. If still occuring I'll attempt to put together a repo to reproduce. |
@rcolwell-cb I haven't had chance to look into this one yet but will shortly, but based on what you're saying it's possible it could be related to this: #26490 |
I had tested that code from the PR you referenced above just now - it does not resolve the issue I'm seeing. The nx release command still produces 10 package updates, all have correct dependency updates - but only 6 include bumped versions / changelogs. Would expect 10 package updates / 10 bumped versions/changelogs etc - with independent / updateDependents config. |
Ok thanks for confirming, hopefully you can reproduce by replicating the graph structure of those 10 packages in the aforementioned repo |
I have reproduced this on my end in the NextUI repo. So I forked the nextUI repo, which has a lot of components and was seeming to use turbo. You'll see in the few commits the changes made necessary to reproduce. I set pnpm 9 in my forked root package.json because have to... I edited the button, updated some constraints, had to turn off project changelogs (youll see another random error if you enable the changelogs in nx.json, cannot read undefined var 'newVersion' - didn't experience that error in my projects however.) The behavior is the same where in some cases not all of the dependency updates result in an update of the package itself. Dependency updates due to button update occur to 18 packages, but only 8 packages bumped. rcolwell-cb/nextui-nx-test@c6bb03c To retest I believe you could simply clone, pnpm install, go back to prior commit, delete tags, then rerun 'pnpm nx release (--dry-run of course first and look at output)' to reproduce. |
@JamesHenry not sure if you saw update - but can be reproduced the issue with a more complex repo above. |
This issue has been automatically marked as stale because no reproduction was provided within 7 days. |
I provided a repository above, and another user has experienced a similar issue. This feature is still broken with particular configurations. |
Thanks for your patience on this @rcolwell-cb, we have been engaged in some company planning so I haven't had chance to loop back to it yet. I will make sure I can repro using your provided instructions today and revert back if not, thanks again |
Howsit guys, I have been following this ticket and am also struggling with the same issue. My situation is as follows: The changes to shared-ui are being done in a branch. The list of affected apps is correct; $ nx show projects --type app --affected
sales-orders-web
company-web but Even explicitly listing the projects to release yields the same output, ie: $ nx release --projects sales-orders-web company-web --dry-run I've followed @JamesHenry demo, but get same unsuccessful results. I think this may be due to the differences between my setup and his, James' packages are all publishable, where as in my repo, only the apps are published, so the libs dont have Thank, G |
@JamesHenry a fix is always better than a revert if it can be figured out! I haven't updated this repo to latest yet but should be easy enough of course. Or if for some reason my fork gives issues - fork the nextui repo and follow similar changes I made in my repo. if(projectToDependencyBumps.has(projectName)) (ln 236 at that time) is where I placed breakpoint then looped through processing of packages. projectToDependencyBumps preferably needs populated somehow before the big loop. |
@rcolwell-cb When I said revert back, I meant in the sense of conversationally on this thread, not in terms of reverting code, there isn't a specific code change that can be reverted in this case. I would not class what you have provided as a minimal reproduction I'm afraid, I am struggling to unpick things, there are a lot of moving parts here which are not relevant to the issue, and your instructions of going back a commit do not hold true. I had a quick go on Friday but was very tired so ascribed it to that and bumped to today with fresh eyes, however I am still struggling. Please kindly work on a minimal reproduction where step by step instructions can be follow to reproduce the issue, that's still the outstanding request please. You reiterate several times that it should be "easy enough to reproduce" but still steps to do so aren't available, please help me to help you |
@ksdc-grantw When you say unsuccessfully, do you mean that in the sense that you were not able to reproduce on the fresh repo? |
Actually @ksdc-grantw in re-reading your comment, it explains it fully. The JS release logic currently operates based on package.json relationships, so the fact you are missing the relationships in a package.json, such as for example: apps/sales-orders-web/package.json {
"name": "@release-repro/sales-orders-web",
"version": "0.0.5",
"dependencies": {
"@release-repro/sales-orders": "0.0.4",
"@release-repro/shared-ui": "0.0.3"
}
} apps/shared-ui/package.json {
"name": "@release-repro/shared-ui",
"private": true,
"version": "0.0.3"
} ...explains why you are not getting any dependent version bumps happening. We do not currently have the concept of versioning a JavaScript library that does not have a package.json (in the same way that other release tools do not). So yeah the resolution in your case is to add missing package.json files and dependency relationships and let Nx update them for you. @rcolwell-cb is there a chance that your situation is related? Is it possible you have missing dependency relationships in package.json files? |
@rcolwell-cb Referenced this issue on in a comment on my issue here #26592 (comment) I've been following this thread and I don't think the issue lies in having an incorrect package.json setup: This initial commit 'publishes' all packages (just tag for sake of simplicity): 52d42b5ffeb856c99a007fe036f7f0e6e21233ea and it's associated action: https://github.com/Warrper/nx-repo-example/actions/runs/9659001290/job/26641818707 This commit then makes a change to package-a which should theoretically bump it's dependents: As you can see, only package-a gets bumped by this change when it should be also bumping its dependents |
@Warrper Thanks a lot for providing the repo. In your case, it is simply that you are using the previous major version of Nx. When I update your repo to use the latest Nx I did, however, discover an issue with the changelog output in this particular setup which is patched here: #26671 so again, thanks for the repo, it was very useful. ❯ nx release -d
NX Running release version for project: @wp-test/test-package-a
@wp-test/test-package-a 🔍 Reading data for package "@wp-test/test-package-a" from packages/folder-a/package-a/package.json
@wp-test/test-package-a 📄 Resolved the current version as 1.2.0 from git tag "@wp-test/test-package-a@1.2.0".
@wp-test/test-package-a 📄 Resolved the specifier as "patch" using git history and the conventional commits standard.
@wp-test/test-package-a ✍️ New version 1.2.1 written to packages/folder-a/package-a/package.json
@wp-test/test-package-a ✍️ Applying new version 1.2.1 to 1 package which depends on @wp-test/test-package-a
NX Running release version for project: @wp-test/test-package-e
@wp-test/test-package-e 🔍 Reading data for package "@wp-test/test-package-e" from packages/folder-b/package-e/package.json
@wp-test/test-package-e 📄 Resolved the current version as 1.1.0 from git tag "@wp-test/test-package-e@1.1.0".
@wp-test/test-package-e 🚫 No changes were detected using git history and the conventional commits standard.
@wp-test/test-package-e 🚫 Skipping versioning "@wp-test/test-package-e" as no changes were detected.
NX Running release version for project: @wp-test/test-package-d
@wp-test/test-package-d 🔍 Reading data for package "@wp-test/test-package-d" from packages/folder-b/folder-c/package-d/package.json
@wp-test/test-package-d 📄 Resolved the current version as 1.1.0 from git tag "@wp-test/test-package-d@1.1.0".
@wp-test/test-package-d 📄 Resolved the specifier as "patch" because "release.version.generatorOptions.updateDependents" is enabled
@wp-test/test-package-d ✍️ New version 1.1.1 written to packages/folder-b/folder-c/package-d/package.json
NX Running release version for project: @wp-test/test-package-c
@wp-test/test-package-c 🔍 Reading data for package "@wp-test/test-package-c" from packages/folder-b/folder-c/package-c/package.json
@wp-test/test-package-c 📄 Resolved the current version as 1.1.0 from git tag "@wp-test/test-package-c@1.1.0".
@wp-test/test-package-c 📄 Resolved the specifier as "patch" because "release.version.generatorOptions.updateDependents" is enabled
@wp-test/test-package-c ✍️ New version 1.1.1 written to packages/folder-b/folder-c/package-c/package.json
NX Running release version for project: @wp-test/test-package-b
@wp-test/test-package-b 🔍 Reading data for package "@wp-test/test-package-b" from packages/folder-b/package-b/package.json
@wp-test/test-package-b 📄 Resolved the current version as 1.1.0 from git tag "@wp-test/test-package-b@1.1.0".
@wp-test/test-package-b 📄 Resolved the specifier as "patch" because "release.version.generatorOptions.updateDependents" is enabled
@wp-test/test-package-b ✍️ New version 1.1.1 written to packages/folder-b/package-b/package.json
UPDATE packages/folder-a/package-a/package.json [dry-run]
"name": "@wp-test/test-package-a",
- "version": "1.0.0",
+ "version": "1.2.1",
"keywords": [],
UPDATE packages/folder-b/folder-c/package-c/package.json [dry-run]
"name": "@wp-test/test-package-c",
- "version": "1.0.0",
+ "version": "1.1.1",
"keywords": [],
"dependencies": {
- "@wp-test/test-package-a": "1.0.0",
+ "@wp-test/test-package-a": "1.2.1",
"@wp-test/test-package-e": "1.0.0"
UPDATE packages/folder-b/folder-c/package-d/package.json [dry-run]
"name": "@wp-test/test-package-d",
- "version": "1.0.0",
+ "version": "1.1.1",
"keywords": [],
"dependencies": {
- "@wp-test/test-package-a": "1.0.0"
+ "@wp-test/test-package-a": "1.2.1"
}
UPDATE packages/folder-b/package-b/package.json [dry-run]
"name": "@wp-test/test-package-b",
- "version": "1.0.0",
+ "version": "1.1.1",
"keywords": [],
"dependencies": {
- "@wp-test/test-package-a": "1.0.0",
- "@wp-test/test-package-c": "1.0.0"
+ "@wp-test/test-package-a": "1.2.1",
+ "@wp-test/test-package-c": "1.1.1"
}
NX Staging changed files with git
NX Committing changes with git
NX Tagging commit with git
NX Pushing to git remote
NX Creating GitHub Release
CREATE https://github.com/Warrper/nx-repo-example/releases/tag/@wp-test/test-package-c@1.1.1 [dry-run]
+ ## 1.1.1 (2024-06-25)
+
+
+ ### 🧱 Updated Dependencies
+
+ - Updated @wp-test/test-package-a to 1.2.1
NX Creating GitHub Release
CREATE https://github.com/Warrper/nx-repo-example/releases/tag/@wp-test/test-package-d@1.1.1 [dry-run]
+ ## 1.1.1 (2024-06-25)
+
+
+ ### 🧱 Updated Dependencies
+
+ - Updated @wp-test/test-package-a to 1.2.1
NX Creating GitHub Release
CREATE https://github.com/Warrper/nx-repo-example/releases/tag/@wp-test/test-package-a@1.2.1 [dry-run]
+ ## 1.2.1 (2024-06-25)
+
+
+ ### 🩹 Fixes
+
+ - **@wp-test/test-package-a:** test change ([87d3528](https://github.com/Warrper/nx-repo-example/commit/87d3528))
+
+
+ ### ❤️ Thank You
+
+ - “JamesHenry” @JamesHenry
NX Creating GitHub Release
CREATE https://github.com/Warrper/nx-repo-example/releases/tag/@wp-test/test-package-b@1.1.1 [dry-run]
+ ## 1.1.1 (2024-06-25)
+
+
+ ### 🧱 Updated Dependencies
+
+ - Updated @wp-test/test-package-c to 1.1.1
+ - Updated @wp-test/test-package-a to 1.2.1
NX Skipped publishing packages.
NOTE: The "dryRun" flag means no changes were made. |
Thanks @JamesHenry , I've applied the changes you recommended. And things are working great, apps are updating when dependent libs change, fantastic. Initailly I was also only running the release command for $ git tag
release/company-web/0.1.0
...
release/company-web/0.2.5
release/feat-companies/0.1.0
release/feat-companies/0.2.0
release/feat-sales-orders/0.1.0
release/sales-order-web/0.1.0
...
release/sales-order-web/0.2.3
release/shared-ui/0.1.0
release/shared-ui/0.1.1
release/shared-ui/0.1.2 You can see that the feat-* and shared libs are also tagged. Is there a way to only tag specifc modules, like Thanks again, |
This issue has been automatically marked as stale because no reproduction was provided within 7 days. |
Being having the same issue and thanks for helping @JamesHenry
|
This issue has been automatically marked as stale because no reproduction was provided within 7 days. |
I'm seeing an issue that seems related to this, but I'm not sure if it's a separate issue. I have a project that is set up as follows: In this case, I would expect that when a project is updated, the other projects it is dependent on are not updated- however this is not the case and I am occasionally seeing dependents be updated. From poking around the code, I noticed that this if block does not check the state of update dependents:
but this block does:
In the cases where I have projects that are unexpectedly publishing, it is going through the first block. Adding the additional check in my local environment prevents the unexpected packages from publishing- but I wanted to know if this was intentional or a bug to be fixed. |
@jlaminate it might be a bug, but please create a minimal reproduction based on your repo so that we can be concrete and create a failing e2e test based on it |
Sure! I'll get back to you with a minimum reproduction library. |
@JamesHenry |
This issue has been automatically marked as stale because no reproduction was provided within 7 days. |
I have a branch with a fix for my issue, but I'm having some trouble finding examples of unit tests involving conventional commits. I can show that it works on my reproduction reproduction - is that sufficient for a PR or would you like some additional unit tests, as well? |
This issue has been automatically marked as stale because no reproduction was provided within 7 days. |
Thanks a lot for the repro @jlaminate, I'm sorry for the delay, I'll get this patched |
I have opened PR #27851 to address it. This issue has been open for quite some time and has evolved multiple times, so I am going to close it as it is becoming hard to follow (and all the known/reproducible issues have been covered). @BioCarmen I'm afraid I simply don't understand your comment/question, please open a new issue with full context and steps to reproduce. For anyone else with their own issues, please follow the same advice. Thanks all! |
This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context. |
Current Behavior
The current behavior I am experiencing is that the "projectToDependencyBumps" in release-version.ts is not getting populated until it processes that project. This then means that it is only updating dependent projects in the repo which are processed after projectToDependencyBumps is populated. There needs to be some sort of sorting going on as its processed, I just spent a long time stepping through with my project.
In my expected behavior below - what's happening is datepicker and dropdown get dependencies updated, but not a new release until button is processed. Once button is processed, projectToDependencyBumps is populated correctly with the packages I intend to update, the rest of the affected packages modal and filter successfully update both the dependency of the button and a new version/changelog.
Unless there is a flag controlling how this is processed or I'm missing something - this feature is currently broken in the beta.
@JamesHenry
@FrozenPandaz
Expected Behavior
I have a mono repo:
@org/my-project
w/ packages:
@org/myproject-datepicker
@org/myproject-dropdown
@org/myproject-button
@org/myproject-modal
@org/myproject-filter
@org/myproject-otherpackages* (these are other packages that don't have @org/my-project-button as a dependency, and don't get affected)
all the other components have button as a dependency.
I make an update to myproject-button/src/button.tsx - commit w/ conventional commits style.
I run nx release -- its a perfect world, it processes everything, has no problems and I see date picker, dropdown, modal, and filter w/ updated dependency of the button and a new version of each released.
GitHub Repo
No response
Steps to Reproduce
Nx Report
Failure Logs
No response
Package Manager Version
No response
Operating System
Additional Information
No response
The text was updated successfully, but these errors were encountered: