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

nx-release-publish default target dependsOn is ignored #27749

Closed
4 tasks
ThePlenkov opened this issue Sep 3, 2024 · 9 comments · Fixed by #27764
Closed
4 tasks

nx-release-publish default target dependsOn is ignored #27749

ThePlenkov opened this issue Sep 3, 2024 · 9 comments · Fixed by #27764
Assignees

Comments

@ThePlenkov
Copy link

Current Behavior

I would like to make sure that project is built, tested and linted before publishing. To achieve this I maintain a default target in nx.json like this

 "nx-release-publish": {
      "options": {
        "packageRoot": "dist/packages/{projectName}"
      },
      "dependsOn": [
        "build",
        "lint",
        "postbuild"
      ]
    },

However when I run a command nx release those tasks are ignored.

So I noticed that in a project graph which I get with nx graph --file=graph.json command my projects have a target nx-release-publish, but dependsOn is overwritten with dependsOn: ['^nx-release-publish'].

I quickly found a place in a code which creates this problem:
in the file node_modules/nx/src/utils/package-json.js

 /**
     * Add implicit nx-release-publish target for all package.json files that are
     * not marked as `"private": true` to allow for lightweight configuration for
     * package based repos.
     */
    if (!isPrivate && !res['nx-release-publish']) {
        res['nx-release-publish'] = {
            dependsOn: ['^nx-release-publish'],
            executor: '@nx/js:release-publish',
            options: {},
        };
    }

Commenting dependsOn line in this code fully solves my problem. I'd like to propose a PR to elaborate on this apporach

Expected Behavior

If default target is maintained - it should take dependsOn from that target as it works for the rest of the project graph. I propose not to make nx-release-publish such a special target.

GitHub Repo

No response

Steps to Reproduce

  1. Add dependsOn for nx-release-publish in nx.json. For example build
  2. Try to release, project will not be built

Nx Report

Node           : 20.16.0
OS             : linux-x64
Native Target  : x86_64-linux
npm            : 10.8.1

nx (global)        : 19.6.4
nx                 : 19.6.3
@nx/js             : 19.6.3
@nx/jest           : 19.6.3
@nx/linter         : 19.6.3
@nx/eslint         : 19.6.3
@nx/workspace      : 19.6.3
@nx/devkit         : 19.6.3
@nx/eslint-plugin  : 19.6.3
@nx/plugin         : 19.6.3
@nx/rollup         : 19.6.3
@nrwl/tao          : 19.6.3
@nx/vite           : 19.6.3
@nx/web            : 19.6.3
typescript         : 5.5.4
---------------------------------------
Registered Plugins:
@nx/eslint/plugin
@nx/jest/plugin
---------------------------------------
Local workspace plugins:
         workspace-plugin

Failure Logs

No response

Package Manager Version

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

No response

@ThePlenkov
Copy link
Author

#22720 #21855 could be solved with this issue as well

@ThePlenkov
Copy link
Author

I found also a different "hacky" way to get same, by adding a dummy section to each project package.json:

  "nx": {
    "targets": {
      "nx-release-publish": {}
    }
  }

to my opinion there must be no difference between this task and others. If we create a proper task inference plugin , there will be no need in this extra step.

@JamesHenry
Copy link
Collaborator

The fact that we run a target called nx-release-publish should be considered an implementation detail of nx release.

We can do a much better job of not having our configuration expose this implementation detail so strongly (the custom packageRoot is the only reason it needed to be exposed at all), I will work on that very soon and provide a relevant migration.

Until then, I have provided the recommended guidance for running dependent tasks before releases in other issues but I will repeat them here and add them to the documentation until the above change lands.


To me, publishing is the act of taking an already finalized artifact and adding it to a registry. It is not the point at which you prepare that relevant artifact.

This is why we have the version.preVersionCommand hook available in nx release config, to aid with performing steps other than simply updating version numbers as part of the preparation of the artifact to publish.

You do have two additional mechanisms available to you as well, however:

  • Use the programmatic API https://nx.dev/features/manage-releases#using-the-programmatic-api-for-nx-release
    • This is a first-class use-case (i.e. not a workaround). Using the programmatic API allows you to weave any custom logic you want between steps of the release process, including running building, linting or whatever else.
  • Use vanilla npm lifecycle scripts (not my recommendation personally, because of my above sentiment on what publishing is, but it works)
    • If you have a prepare or prepublishOnly script in your package.json, it will be invoked automatically because behind the scenes the nx-release-publish target is invoking the npm CLI directly.

@ThePlenkov
Copy link
Author

ThePlenkov commented Sep 4, 2024

Unfortuanately preversion command will not work in our case because of a simple reason:

  • we isssue the version locally and that's not a problem there, usually project is built
  • we publish newly released versions via CI/CD which is triggered via tag pipelines
  • in this case if we only do nx release publish preVersionCommand is not executed anymore
  • since project is not built - publish task will fail
  • the case is - we can run build of course manually - but then again we're delegating the project graph configuration to CI/CD which is not fully right too and not using a core nx feature.

With a change introduced by this PR the only required command in CI/CD is : npx nx release publish

With properly maintained dependencies it will execute all required tasks right in CI/CD.

@ThePlenkov
Copy link
Author

@JamesHenry following mentioned above problem what could be a workaround solution ( at least to stay consistend with the current approach ) is kind of prepublish command which will be called prior to publish, not version

@ThePlenkov
Copy link
Author

Another idea which I really like about having release/publish as interference plugin, is that in this case we can utilze the full scope of commands, like nx run, nx run-many, nx affected. Like here I started a discussion: #27758

It would be nice to have something like:

nx affected publish ( where publish has a dedicated executor like @nx/release-publish ) and then we can just use it as a task, not as a separate functionality. We may reach better flexibility .

@JamesHenry
Copy link
Collaborator

JamesHenry commented Sep 4, 2024

@ThePlenkov both of the additional options I outlined will work in your case, the npm scripts are also part of the project graph, for example. There is nothing wrong with these solutions, as you seem to be implying.

Nevertheless, I have opened a PR with a more appropriate update to the implicit target logic (commenting out a line of code which was important to functionality ensuring correctness was never going to be a viable solution). It is in draft pending discussion with the team: #27764


RE: Affected:

Affected as a mechanism would only say that a project is affected within a particular commit range.

You could use this to infer what needs to be released, but it wouldn't say what the new version should be. That's why we have different options for determining the new version: imperatively via the CLI, conventional commits, and version plans.

Conventional commits and version plans already determine what is "affected" based on the semver bump data that is encoded within them.

We could potentially explore what an affected imperative versioning would look like, but again, it's a question of what the bump should be. Some projects could be "affected" but want a patch, whereas others want a minor release.

Having only publishing be combined with affected is also problematic, because again, affected refers to a project being impacted by git changes, which is not necessarily related to the desire to publish, because the artifact you are publishing is usually mostly outside git (entirely in some cases, like the Nx repo where we version and publish from dist and use the registry as the source of truth for versioning).

Nx release needs to generically support a large number of different approaches, so we can't just add things without considering the ripple effect for all use-cases.

@ThePlenkov
Copy link
Author

I really like this overview BTW. It shows all tasks which were performed on publish. Exactly what I wanted
image

Copy link

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.

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

Successfully merging a pull request may close this issue.

2 participants