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: allow restoring replaced workspace:/file: references after publishing #20978

Open
1 task
virtuallyunknown opened this issue Jan 2, 2024 · 31 comments

Comments

@virtuallyunknown
Copy link

virtuallyunknown commented Jan 2, 2024

Description

Greetings. I just started learning with nx a few days ago, so in case I'm missing something obvious, sorry in advance.

That said, I've noticed that the nx release publish command is using npm under the hood to handle publishing to the registry, even if you are in a pnpm workspace. Unfortunately, this has some unintended side effects in relation to this particular pnpm feature:

When a workspace package is packed into an archive (whether it's through pnpm pack or one of the publish commands like pnpm publish), we dynamically replace any workspace: dependency by:

  • The corresponding version in the target workspace (if you use workspace:*, workspace:~, or workspace:^)
  • The associated semver range (for any other range type)

https://pnpm.io/workspaces#publishing-workspace-packages

In other words, if you're using the so called pnpm "workspace protocol" for internal dependencies in your monorepo, npm will pack an archive containing the word workspace: as a value for dependency versions (which is not valid for other package managers), instead of dynamically replacing these values with the appropriate versions like pnpm does.

Motivation

I think nx being able to detect you're in a pnpm workspace (I think it already does that?) and then prefer the pnpm's equivalent of publish would be beneficial for people who rely on the feature I linked to above.

Alternatively, it would be even better if nx made this a configurable options or a cli parameter, similar to how registry works.

Cheers!


EDIT: Okay so I actually just noticed that the nx release version command which I use before publish is replacing all workspace: version occurrences with the actual version string for internal dependencies. I suppose this is intended, but then how do I make use the workspace protocol, or should I even do that?

I also noticed that this commands reformats the package json files and changes all indentation, is there any way to disable it?

@JamesHenry
Copy link
Collaborator

JamesHenry commented Jan 3, 2024

@virtuallyunknown I'm afraid it isn't documented yet, but right now for consistency reasons we have basically two different approaches when it comes to versioning dependencies:

  • update your source files with explicit versions and track them over time in git
  • use workspace:/file: protocols and version your dist files instead (meaning the versions are not tracked in git over time). This is how the nx repo itself does things. The source of truth for the current version in this case is the npm registry, rather than files on disk. We like this approach because there is always a place you can look at on disk for exactly what you published to the registry, which is lost in some other tools that do the substitution in memory

Having said that, we now have some extra capabilities in nx release which might make it possible for users to opt into a blended approach if that's what they choose to do (even though they lose that advantage I mentioned).

I will leave this issue open to track

@JamesHenry JamesHenry changed the title nx release publish: prefer pnpm over of npm internally in a pnpm workspace to publish packages to the registry. nx release: allow restoring replaced workspace:/file: references after publishing Jan 3, 2024
@virtuallyunknown
Copy link
Author

@JamesHenry thanks for the reply. I actually watched your YouTube video on nx release from the nx conf, this is what got me interested in nx in first place, so it's a pleasure to meet you!

About your comment - the first approach seems pretty self-explanatory, but in the 2nd approach where you use the NPM registry, what happens if for example one of your projects is set to private (private as in not published on any registry)? What would be the source of truth for the version number in that case?

I think for the time being I will stick with the first approach with explicit versions in the package.json files, and maybe later on when more features and documentation is added I will migrate. It is not mission critical for me to use the workspace protocol, so no big deal I suppose.

Just a follow up question, is it possible to prevent nx release version from re-formatting/indenting my package.json files? If it isn't, may I suggest/request this as a feature? I could also open a separate issue about it if that would be of any help.

Cheers!

@JamesHenry
Copy link
Collaborator

@virtuallyunknown are you packages released independently?

Regarding the versioning performing formatting, it should only perform formatting based on your configured prettier installation. If you do not want your package.json files to be formatted, you should be able to add them to your .prettierignore file

@virtuallyunknown
Copy link
Author

virtuallyunknown commented Jan 5, 2024

@JamesHenry, I am using "fixed" versioning for the time being.

Regarding the versioning performing formatting, it should only perform formatting based on your configured prettier installation. If you do not want your package.json files to be formatted, you should be able to add them to your .prettierignore file

Is there any prior setup for prettier required for the .prettierignore file to get recognized by nx? Because in my current project I wasn't using prettier at all actually.

I did follow your advice and added a .prettierignore file with package.json files being ignored, but it didn't work. I then installed prettier itself and added the following configuration in project root's .prettierrc file:

{
    "tabWidth": 12
}

This actually worked, however I don't want prettier in my project, and it seems like the .prettierignore file is ironically the one that is being ignored by nx release version.

@JamesHenry
Copy link
Collaborator

JamesHenry commented Jan 5, 2024

Ok that seems strange then, @fahslaj will take a look, do you have a repo you can share?

UPDATE: If I had to guess, I would say it isn't actually formatting per se, it is just standard JSON serialization and deserialization happening as we patch the JSON data in the file. The vast majority of our userbase use prettier to standardize their formatting so that there is no manual enforcement required, which is why we automatically invoke it whenever it is available and this becomes a non-issue

@virtuallyunknown
Copy link
Author

Ok that seems strange then, @fahslaj will take a look, do you have a repo you can share?

Yes, here it is actually, I was about to make an edit:

https://github.com/virtuallyunknown/nx-monorepo

It's very similar to the one I created for #20993. A dry run can be performed via pnpm exec tsx release.ts.

@JamesHenry
Copy link
Collaborator

Hopefully you saw my updated comment above, I posted it at the same time

@virtuallyunknown
Copy link
Author

virtuallyunknown commented Jan 5, 2024

Hopefully you saw my updated comment above, I posted it at the same time

So if I'm understanding correctly, what you're saying is that .prettierignore plays no role actually, and it's the JSON serialization and deserialization that messes up with my indenting. Then, if prettier is available in an nx monorepo, you use it to apply formatting.

Or in other words, I need prettier to get around this problem, or alternatively do some post-processing of the package.json files on my own (right after nx release version runs)?

EDIT: Ok, yep. Apparently if you add prettier as a dependency then nx detects that and it will format the package.json files according to the specified `.prettierrc. config. Not ideal for me because I need an extra dependency, and it also messes with my global prettier configuration (coming from the VSCode extension), but I understand I'm probably in the tiny minority that will ever have such problem here so it is what it is.

Thanks again for helping out.

@JamesHenry
Copy link
Collaborator

@virtuallyunknown yes exactly.

So it sounds like you do value automated, deterministic formatting if you are using prettier via vscode... I have honestly never encountered a set up where you do seem to care about formatting but don't want to enforce that formatting on your project (and just run it ad hoc via your editor). Usually folks either use prettier in their project and editor, or not at all.

Are all your projects currently only single developer? i.e. you don't need to worry about other people contributing using different formatting given it's not being checked or enforced at the project level?

I think you're right that this is an edge case for that reason and probably not something that will be addressed within nx release

@virtuallyunknown
Copy link
Author

@JamesHenry you are correct, I work as a single developer for the vast majority of the time and as such, the vscode extension is a good solution since you can have one global config file that applies everywhere. All formatting is basically enforced by the editor on save, and I don't have to pollute the project directory space with unnecessary config files. Though obviously this is not a good solution for collaborative projects.

I am also not a big fan of prettier, even thought I understand the value it brings, so maybe I will even migrate to dprint once I have the time to look into it. Either way, not to make this too long, what you're saying makes total sense and I agree nx shouldn't try and cover every edge case out there. All that said, nx doesn't force any formatter (or not formatter) upon me so I can just work around the issue, or alternatively just install prettier.

Please feel free to close the issue necessary. Cheers!

@JamesHenry
Copy link
Collaborator

Cool thanks @virtuallyunknown makes sense! We will keep the issue open because it primarily relates to the restoration of workspace/file references in source files, which as I mentioned I think is worth reevaluating now that we have the all in onenx release command and the programmatic API

@isshesure
Copy link

What is the recommendation here? It seems most monorepo users are using Yarn or pnpm, and most are using the workspace protocol?

It seems then that you cannot publish any packages using the Nx tooling, including the nx release CLI, the NX semantic release plugin, or the Nx semver plugin, as they all use npm publish under the hood?

The recommendation is to implement the somewhat convoluted (with dependencies manually maintained) solution that the Nx repo uses?

Or are you expecting the nx release CLI to soon detect the package manager and use a yarn/pnpm publish instead?

@JamesHenry
Copy link
Collaborator

@isshesure my comment above covers the current case with workspace protocol:

#20978 (comment)

use workspace:/file: protocols and version your dist files instead (meaning the versions are not tracked in git over time). This is how the nx repo itself does things. The source of truth for the current version in this case is the npm registry, rather than files on disk. We like this approach because there is always a place you can look at on disk for exactly what you published to the registry, which is lost in some other tools that do the substitution in memory

The relevant configuration on the nx repo to make this work is here:

nx/nx.json

Lines 41 to 53 in 69523f1

"version": {
"generatorOptions": {
"packageRoot": "build/packages/{projectName}",
"currentVersionResolver": "registry"
}
}
},
"targetDefaults": {
"nx-release-publish": {
"options": {
"packageRoot": "build/packages/{projectName}"
}
},

You will need to set the custom packageRoot based on what makes sense for your workspace.

As stated in that comment, we are going to look again at this case now that the nx release tooling has evolved and has more capabilities.

Technically if you wanted though I believe you could already script this to work on your source using the programmatic API. The version command returns an object covering the versions before and after, so you could use that to restore the files back to the workspace protocol after publishing. Again, I'm not saying this is as good as it can be, but that is the current workable state.

Hope that helps for now, follow along with this issue for updates on both the docs for this and the improved DX that @fahslaj is working on

@eddienubes
Copy link

Hi! Just faced this behaviour.
nx replaced all workspace: protocols with versions without restoring them.
This is a very confusing and frustrating workflow. Especially when migrating from tools like lerna.

I didn't provide any valuable input, but hope to somehow facilitate the complete support for yarn and pnpm publishing.

@eddienubes
Copy link

eddienubes commented Apr 20, 2024

👆 Adding to my recent comment:
After running nx release version. nx/js plugin besides incrementing and replacing workspace: protocols also runs pnpm install for some reason.

Is it expected? I also wouldn't say I'm experienced with nx, but I'm trying to find a complete tool for managing monorepo's builds, releases and versioning.
It's quite messy to dive into 3 different tools at once:

  • pnpm for dependency management across workspaces
  • nx for building and caching.
  • changesets/lerna for versioning and publishing.

But after discovering nx release command I was hoping to eliminate the need for the 3rd one.

Please, let me know If nx is the right tool here, maybe I have to stick to changesets/lerna for this kind of task.

Thanks!

Dump:

repo link: https://github.com/eddienubes/carnytools

UPDATE packages/styles/package.json

    "name": "@carnytools/styles",
-   "version": "0.0.1",
+   "version": "0.0.2",
    "private": false,

UPDATE packages/types/package.json

    "name": "@carnytools/types",
-   "version": "0.0.1",
+   "version": "0.0.2",
    "private": false,

    "devDependencies": {
-     "@carnytools/styles": "workspace:*"
+     "@carnytools/styles": "0.0.2"
    }


 NX   Updating pnpm lock file


 NX   Error updating lock file with command 'pnpm install --lockfile-only'

Verify that 'pnpm install --lockfile-only' succeeds when run from the workspace root.
To configure a string of arguments to be passed to this command, set the 'release.version.generatorOptions.installArgs' property in nx.json.
To ignore install lifecycle scripts, set 'release.version.generatorOptions.installIgnoreScripts' to true in nx.json.
To disable this step entirely, set 'release.version.skipLockFileUpdate' to true in nx.json.


 NX   Command failed: pnpm install --lockfile-only

@recallwei
Copy link

recallwei commented Apr 21, 2024

Face the same.

image

I want to make a v0.0.18 release. One of the packages depends on another package. I use the workspace protocol, but the version script will automatically update it to 0.0.18. However, 0.0.18 has not been released yet.

@ruscon
Copy link

ruscon commented Apr 28, 2024

The same issue.

The solution proposed here looks more like a hack than a normal solution.

@eddienubes
Copy link

@JamesHenry Please let us know if there is anything we can do.

@fahslaj fahslaj added scope: release and removed scope: core core nx functionality labels May 14, 2024
@JamesHenry
Copy link
Collaborator

JamesHenry commented May 21, 2024

Hi Folks,

Just to give an update on this one and a recap of why things behave the way they do right now:

During the early stages, nx release started life as three distinct "subcommands": nx release version, nx release changelog, nx release publish. Over time they have become more and more configurable and pluggable.

A key thing to note about nx release publish is that it is powered by the npm publish CLI, it does not perform any manipulation of your packages in memory before sending them to npm (unlike some other tools). This was a design goal - with nx release you always have a location on disk where you can inspect exactly what your package looked like when it was sent to npm, so there is a lot less room for subtle differences/bugs.

One of the most powerful features of nx release is that these steps of the release process can be invoked via a programmatic API (https://nx.dev/features/manage-releases#using-the-programmatic-api-for-nx-release) instead of using them via the CLI. This allows for developers to work around any features that don't yet exist and customize the behaviour however they see fit.

Later, we added the overall nx release command to the CLI as well which runs all three steps together (unless publishing is skipped via a flag).

When dealing with any non semantic versioning references in package.json files (such as file: or workspace:), the issue is that they need to be updated to actual semantic versions as part of versioning so that they are in a publishable state on disk (see my callout above about how nx release publish is designed to work).

As folks have encountered, there is currently no automated way to revert these updated values back to the file: or workspace: refs after the separate publishing step takes place. Therefore, to date, the two approaches folks take is to either:

  • Use file:/workspace: references and then version and publish dist files (this is what the Nx repo itself does)
  • Use semantic versions in source files and version and publish them directly

I can appreciate that people do want the best of both worlds here, they want to be able to use file:/workspace: in source files and still version and publish them directly.

I have therefore begun prototyping a utility to apply the restoration of the references after the publishing step takes place. It's functional already, I just want to make sure the DX is optimal.

NOTE: It will not be possible to support this via the individual use of the CLI subcommands because it require passing around too much data, however it should be supportable for folks who use the overall nx release CLI command, or the programmatic API.

I will share an update on here once I am happy with the DX and the utility/option is available.

Many thanks!

@pkuczynski
Copy link
Contributor

pkuczynski commented Jun 23, 2024

hey @JamesHenry I just stumbled upon this issue myself and seems there is no resolution for it so far. Is there any update on the utility you mentioned above? Or maybe in the meantime an option in nx come up to life which would preserve "workspace:"?

@pkuczynski
Copy link
Contributor

pkuczynski commented Jun 23, 2024

In the meantime I come up with following workaround in my pipeline in GitHub Actions:

      - run: pnpx nx release --yes

      # Workaround for https://github.com/nrwl/nx/issues/20978
      - run: |
          sudo apt update
          sudo apt install -y patchutils
          git diff HEAD~1 -- "**/package.json" | grepdiff workspace: --output-matching=hunk --only-match=removals | git apply --reverse  --unidiff-zero --allow-empty
          if [ $(git ls-files -m "**/package.json") ]; then
            git add "**/package.json"
            git commit -m "publish: restore workspace dependencies [skip ci]"
            git push
          fi

It's not perfect since it creates additional commit after publish (only when workspace dependency has been removed), but better then nothing.

@bryanjtc
Copy link

Any update on an official solution?

@ryanbas21
Copy link

Question: wouldn't changing

`npm publish "${packageRoot}" --json --"${registryConfigKey}=${registry}" --tag=${tag}`,
to use the packageManager defined either in the package.json or the nx cli options

According to: https://pnpm.io/workspaces#publishing-workspace-packages

When a workspace package is packed into an archive (whether it's through pnpm pack or one of the publish commands like pnpm publish), we dynamically replace any workspace: dependency by:

The corresponding version in the target workspace (if you use workspace:*, workspace:~, or workspace:^)
The associated semver range (for any other range type)

So wouldn't using pnpm publish, when pnpm is the package manager, automagically fix the versions? Or am I misinterpreting something?

@dan-lee
Copy link

dan-lee commented Aug 23, 2024

We have a similar issue. pnpm also supports overriding exports, etc. in publishConfig. This workflow is also broken with nx because it uses npm to publish.

I am just learning nx, so forgive me if this is a stupid question, but why not at least offer a config option or autodetect the package manager, as @ryanbas21 suggests?

@JamesHenry
Copy link
Collaborator

@ryanbas21 Thanks for pointing out this pnpm feature, I was not aware of it and am looking into it now.

@dan-lee We may well expose it as an option to the existing executor, pending exploration into the differences with npm publish and if we can smooth them out

@wood3n
Copy link

wood3n commented Sep 2, 2024

Maybe npx nx release --skip-publish and pnpm publish -r can solve this problem?

@JamesHenry
Copy link
Collaborator

@wood3n That is only workable if during the versioning step those references are preserved.

I do a combination of these approaches is workable when pnpm is the package manager, because as noted by @ryanbas21 pnpm publish differs from npm publish and does support dynamically replacing the references during its internal packing stage.

Therefore, the first step is open as a PR here: #27787

With the new version generatorOption, you will able to tell nx release that you want all the usual versioning logic to apply, including updating independently released dependents via updateDependents: auto, but the difference being that if dependencies are referenced via workspace: or file: protocols, it will leave them untouched.

We will then be able to make a separate update to the publish executor to use pnpm publish when appropriate for that workspace. Eventually, we can also explore if it makes sense to add custom (much more complex) logic for the other package managers.

@JamesHenry
Copy link
Collaborator

I've taken #27787 further and added the detection for pnpm, and running pnpm publish in that case, which supports dynamically replacing the local package protocols during the publish step

SokratisVidros added a commit to novuhq/novu that referenced this issue Sep 13, 2024
After releasing one of our NPM packages, we need to restore the workspace:* protocol in the package.json for local pnpm module resolution.

For more informartion see nrwl/nx#20978
SokratisVidros added a commit to novuhq/novu that referenced this issue Sep 13, 2024
After releasing one of our NPM packages, we need to restore the workspace:* protocol in the package.json for local pnpm module resolution.

For more informartion see nrwl/nx#20978
LetItRock pushed a commit to novuhq/novu that referenced this issue Sep 13, 2024
After releasing one of our NPM packages, we need to restore the workspace:* protocol in the package.json for local pnpm module resolution.

For more informartion see nrwl/nx#20978
@smukkejohan
Copy link

smukkejohan commented Sep 19, 2024

Shouldn't updateDependents 'never' also result in file: and workspace: references being preserved? This is not the case for us on v19.7.4. Actually it seems updateDependents does not take effect, we keep getting the references replaced by the new version.

We also still end up with a file:. reference in the published npm. I thought #27787 should make sure pnpm publish is used and this is replaced correctly?

Screenshot 2024-09-19 at 14 59 35

nx report from CI that published the above package.

NX   Report complete - copy this into the issue template

Node           : 20.17.0
OS             : linux-x64
Native Target  : x86_64-linux
pnpm           : 9.2.0

nx                 : [19](https://github.com/airlookjs/airlookjs/actions/runs/10941204631/job/30375312611#step:7:20).7.4
@nx/js             : 19.7.4
@nx/jest           : 19.7.4
@nx/linter         : 19.7.4
@nx/eslint         : 19.7.4
@nx/workspace      : 19.7.4
@nx/devkit         : 19.7.4
@nx/eslint-plugin  : 19.7.4
@nx/node           : 19.7.4
@nx/playwright     : 19.7.4
@nrwl/tao          : 19.7.4
@nx/vite           : 19.7.4
@nx/web            : 19.7.4
@nx/webpack        : 19.7.4
typescript         : 5.6.2
---------------------------------------
Registered Plugins:
@nx/eslint/plugin
@nx/playwright/plugin
---------------------------------------
Community plugins:
@nx-tools/nx-container : 6.0.2

https://github.com/airlookjs/airlookjs/actions/runs/10941204631/job/30375312611

@KayVeeCodes
Copy link

Hi there,
Are we sure that this solution works? I am still facing the same issue when trying to run nx release, the "workspace: *" protocol is overtighten after the nx release command

@KayVeeCodes
Copy link

KayVeeCodes commented Nov 8, 2024

For people that get here through search I suggest to see this angular-eslint/angular-eslint@d793c6f until proper documentation lands here. Be sure to add the following into nx.json

"generatorOptions": {
     "preserveLocalDependencyProtocols": true
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests