search

nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.69k stars 2.36k forks source link

`nx release` fails when using a custom `dist` directory, `independent` mode, and `conventional-commits`. #27887

Open therockstorm opened 2 months ago

therockstorm commented 2 months ago

Current Behavior

The Nx generators @nx/node:lib and @nx/js:lib put the dist directory at the repository root, so that's what I'm doing. I'm also using conventional commits for version bumps. Taken together, it means I can't use disk to resolve versions and after following this Nx documentation, I removed the version field from package.json files.

This configuration breaks nx release because it tries to publish every package, but any that haven't changed don't have a version field in the package.json file in the dist directory.

This discussion suggests using nx show projects --affected to pass only affected packages to nx release --projects="lib-a,...", but that solution doesn't work. If I change the nx.json file, for example, nx show projects --affected lists every package, but nx release doesn't update any of them. Instead, it shows, Skipping versioning "lib-a" as no changes were detected.

Aside: while creating the repo to reproduce the issue, I ran into another issue: in a fresh Nx monorepo, I add a library via npx nx g lib lib-a --directory=packages/lib-a and then remove the version field from package.json. This breaks nx release --first-release, throwing Unable to resolve the current version from git tag using pattern "{projectName}@{version}". Falling back to the version on disk of undefined. I worked around this by adding the version back, publishing, and then removing it.

Expected Behavior

I expect that using the Nx generators and following the Nx documentation results in my packages publishing without error.

GitHub Repo

https://github.com/ClipboardHealth/nx-release-repro

Steps to Reproduce

  1. Make a change to the string lib-a returns here.
  2. Commit with git commit --all --message "fix: make a change".
  3. Run nx release --yes and receive an error.

Nx Report

Node           : 22.8.0
OS             : darwin-arm64
Native Target  : aarch64-macos
npm            : 10.8.3

nx                 : 19.6.1
@nx/js             : 19.6.1
@nx/jest           : 19.6.1
@nx/linter         : 19.6.1
@nx/eslint         : 19.6.1
@nx/workspace      : 19.6.1
@nx/devkit         : 19.6.1
@nx/eslint-plugin  : 19.6.1
@nx/plugin         : 19.6.1
@nrwl/tao          : 19.6.1
typescript         : 5.5.4

Failure Logs

NX   Executing pre-version command

 NX   Running release version for project: lib-a

lib-a 🔍 Reading data for package "@clipboard-health/lib-a" from dist/packages/lib-a/package.json
lib-a 📄 Resolved the current version as 0.2.1 from git tag "lib-a@0.2.1".
lib-a 📄 Resolved the specifier as "patch" using git history and the conventional commits standard.
lib-a ✍️  New version 0.2.2 written to dist/packages/lib-a/package.json

 NX   Running release version for project: lib-b

lib-b 🔍 Reading data for package "@clipboard-health/lib-b" from dist/packages/lib-b/package.json
lib-b 📄 Resolved the current version as 0.2.2 from git tag "lib-b@0.2.2".
lib-b 🚫 No changes were detected using git history and the conventional commits standard.
lib-b 🚫 Skipping versioning "@clipboard-health/lib-b" as no changes were detected.

UPDATE dist/packages/lib-a/package.json

    "typings": "./src/index.d.ts",
-   "types": "./src/index.d.ts"
+   "types": "./src/index.d.ts",
+   "version": "0.2.2"
  }
+

 NX   Staging changed files with git

No files to stage. Skipping git add.

 NX   Changelogs are disabled. No changelog entries will be generated

To explicitly enable changelog generation, configure "release.changelog.workspaceChangelog" or "release.changelog.projectChangelogs" in nx.json.

 NX   Committing changes with git

No staged files found. Skipping commit.

 NX   Tagging commit with git

 NX   Running target nx-release-publish for 2 projects:

- lib-a
- lib-b

—————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

> nx run lib-a:nx-release-publish

📦  @clipboard-health/lib-a@0.2.2
=== Tarball Contents ===

120B README.md
223B package.json
29B  src/index.d.ts
199B src/index.js
140B src/index.js.map
40B  src/lib/lib-a.d.ts
175B src/lib/lib-a.js
193B src/lib/lib-a.js.map
=== Tarball Details ===
name:          @clipboard-health/lib-a
version:       0.2.2
filename:      clipboard-health-lib-a-0.2.2.tgz
package size:  812 B
unpacked size: 1.1 kB
shasum:        d549ffb751d1b5df8f39faa32dd2b14b89a670df
integrity:     sha512-PG1XmVvrtm+h9[...]rMUcJ0fnU51zA==
total files:   8

Published to https://registry.npmjs.org/ with tag "latest"

> nx run lib-b:nx-release-publish

npm publish error:
invalid semver:

—————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 NX   Running target nx-release-publish for 2 projects failed

Failed tasks:

- lib-b:nx-release-publish

 NX   One or more of the selected projects could not be published

Pass --verbose to see the stacktrace.

Package Manager Version

10.8.3

Operating System

Additional Information

I've spent weeks on and off trying to get nx release working, any help would be greatly appreciated, @fahslaj @JamesHenry.

therockstorm commented 1 month ago

Any update? We're unable to publish packages using the configuration shown in the ClipboardHealth/nx-release-repro.

Ideally, we'd be able to keep the version field in package.json. It's a required field for NPM, so other tooling expects it to be there. A possible solution:

  1. Nx bumps the version in the "real" package.json and package-lock.json files (or equivalent for other package managers).
  2. Nx commits the changes.
  3. Nx runs a build so the compiled package.json in a dist directory (or similar) gets the new version.
  4. Nx publishes the artifacts in the dist directory.
JamesHenry commented 1 month ago

@therockstorm I am taking a look at this now, will let you know if I have any clarifying questions

JamesHenry commented 1 month ago

Aside: while creating the repo to reproduce the issue, I ran into another issue: in a fresh Nx monorepo, I add a library via npx nx g lib lib-a --directory=packages/lib-a and then remove the version field from package.json. This breaks nx release --first-release, throwing Unable to resolve the current version from git tag using pattern "{projectName}@{version}". Falling back to the version on disk of undefined. I worked around this by adding the version back, publishing, and then removing it.

This makes a lot of sense conceptually, right? Conventional commits works by applying a relative bump to an existing version, and here you have not given it any way to resolve a current version across git tags or the fallback on disk.

Now, having said that, I think we greatly improve the feedback and handling if you end up in this state. And it got me thinking, maybe we can do even better than that, and prompt you if you want to continue right there in the moment by applying 0.0.0 as an assumed current version.

E.g. I've just put this together in the context of your repo:

image

When answering YES:

image
JamesHenry commented 1 month ago

Regarding affected, I'd like to please clarify your expectations and what you feel you are missing from conventional commits in this aspect? Conventional commits should already be the source of truth for what projects are "affected"? Nx affected deals with what projects are affected between a particular commit range, but this doesn't necessarily map to what should be released.

I shared some thoughts on that previously here: https://github.com/nrwl/nx/issues/27749#issuecomment-2328871048

Please could you elaborate on why affected is needed/desirable on top of conventional commits and independent releases?

PS I am kind of blown away by the number of upvotes on this issue in such a short space of time, are they largely colleagues of yours or something? Even a blocking bug (which this isn't) wouldn't usually garner this much attention in this short a time span 🤔

JamesHenry commented 1 month ago

The improvement covered in comment https://github.com/nrwl/nx/issues/27887#issuecomment-2365154621 is open as a PR here https://github.com/nrwl/nx/pull/28034

therockstorm commented 1 month ago

I'm indifferent to "affected" while releasing and agree with the distinctions you bring up. The problem is that following these docs to remove versions and add the two packageRoot configurations results in errors.

I think I got it working by removing release.version.generatorOptions.packageRoot, leaving targetDefaults.nx-release-publish.options.packageRoot, and adding version back to each package.json file. With this setup in CI, I can:

  1. Run nx release --skip-publish to bump all package.json versions and commit to git.
  2. Run npx nx run-many --target=build so package.json files in dist directories pick up the new versions.
  3. Run npx nx release publish to publish dist directories to NPM.

So nx.json becomes:

{
  "release": {
    "git": {
      "commit": true
    },
    "projects": ["packages/*"],
    "projectsRelationship": "independent",
    "version": {
      "generatorOptions": {
        "currentVersionResolver": "git-tag",
        "fallbackCurrentVersionResolver": "disk",
        "specifierSource": "conventional-commits",
        "updateDependents": "auto"
      }
    }
  },
  "nx-release-publish": { "options": { "packageRoot": "dist/packages/{projectName}" } }
}

Maybe the "fix" here is just a documentation update? It would also be helpful if ./node_modules/nx/schemas/nx-schema.json's NxReleaseVersionConfiguration.generatorOptions had the complete set of options specified instead of just "type": "object".

Having gone through the code trying to figure out why it wasn't working, I see just how many configuration permutations nx release is attempting to support. Thanks for the response and the effort involved in building it!

JamesHenry commented 1 month ago

Thanks @therockstorm, I agree these docs could do with a refresh to improve clarity, I will work on that very soon once I add another feature in this area that will also impact the docs.

A couple of points on the above:

Rather than setting the currentVersionResolver and specifierSource generator options individually to facilitate conventional commits, you can specify:

"version": {
  "conventionalCommits": true
}

Which implicitly sets the same things behind the scenes (https://nx.dev/recipes/nx-release/automatically-version-with-conventional-commits).

There reason the schema population for generatorOptions was not done was for consistency. Versioning is intentionally pluggable for all languages/ecosystems not just JavaScript with npm. The generatorOptions are therefore specific to and determined by the generator implementation being used. By default nx release will assume the JS one and load it for you without any additional config, but there is no way in JSON schema to have this level of dynamism. Now having said that, given we have this implied default of the common case because JS + npm, I think there is a way we can build those specific options into the schema and then if version.generator is set to anything but the first party nx JS one, we revert to the current open types we have now. I will look into this now

jakachira commented 1 month ago

Hi @JamesHenry

Thanks for all the efforts with this nx target.

I have question regarding currentVersionResolver if it is set to "registry" can the "specifier-source " still be set to "conventional-commits"?

We have a challenge with git-tags as they can create conflicts between developers' PRs.

Thanks