search

sindresorhus / np

A better `npm publish`
MIT License
7.56k stars 299 forks source link

Duplicate Tag + Release created without `v` Prefix #738

Open karlhorky opened 8 months ago

karlhorky commented 8 months ago

Description

Publishing a new version of our package with np@10.0.1 (with pnpm) caused a missing v prefix on the GitHub release name (observe the missing v in the first URL below).

Digging in deeper reveals that two Git tags were created v1.1.0 and 1.1.0, one with GitHub Release with release notes, one without:

stylelint-config-upleveled Tags:

Logs from the publish:

➜  stylelint-config-upleveled git:(main) np --version
10.0.1
➜  stylelint-config-upleveled git:(main) np

Publish a new version of stylelint-config-upleveled (current: 1.0.7)

Commits:
- Ignore @theme for Tailwind CSS v4  f0701c0
- Update dependency typescript to v5.4.2 (#71)  75e287e
- Update dependency eslint to v8.57.0 (#70)  ddeefe5
- Update dependency upgrades - non-major (#69)  4676887
- Update dependency eslint-config-upleveled to v7.8.0 (#68)  bb095ba
- Update dependency eslint-config-upleveled to v7.7.1 (#67)  bec07e3
- Update dependency upgrades - non-major (#66)  cafded1
- Update dependency stylelint to ^16.2.0 (#65)  8d8847b
- Bump vite from 5.0.8 to 5.0.12 (#64)  09c9051
- Update dependency vitest to v1.2.1 (#63)  492160b
- Update dependency vitest to v1.1.3 (#62)  a80a9d1
- Update dependency upgrades - non-major (#61)  0b6fa47

Commit Range:
v1.0.7...main

Registry:
https://registry.npmjs.org/

? Select SemVer increment or specify new version minor  1.1.0

  ✔ Prerequisite check
  ✔ Git
  ✔ Installing dependencies using pnpm
  ✔ Running tests
  ✔ Bumping version
  ✔ Publishing package
  ✔ Pushing tags
  ✔ Creating release draft on GitHub

 stylelint-config-upleveled 1.1.0 published 🎉

Is this intentional? I could not see this change in the 10.0.0 release notes

Maybe this was something introduced by the pnpm support PR by @mmkal or the followup pnpm commits by @tommy-mitchell?

Previous Behavior

The previous versions of np created tags with the v prefix, such as v1.0.7 here:

Steps to reproduce

  1. Publish new minor version of stylelint-config-upleveled by running np
  2. Accept the default settings in the GitHub Release create form when it loads in the browser
  3. Observe tag created with v prefix (without GitHub Release + release notes)
  4. Observe duplicate tag created without v prefix (and with GitHub Release + release notes)

Expected behavior

Only a single tag + GitHub Release should be created, with a v prefix and containing the release notes

Environment

np - 10.0.1 Node.js - 20.11.1 npm - 10.2.4 pnpm - 8.15.3 Git - 2.39.2 OS - macOS Sonoma 14.4

mmkal commented 8 months ago

Definitely sounds wrong that two separate tags are being created. We use pnpm config get tag-version-prefix for pnpm, which in my case is empty string, whereas npm config get tag-version-prefix is 'v'.

Might be that somewhere is hard-coding a 'v'.

Probably the easiest workaround right now is to set the pnpm config value to 'v', until it's fixed. We probably should keep the no-prefix behaviour for pnpm but definitely shouldn't have two separate tags.

karlhorky commented 8 months ago

Interesting, I think I may have found more information - during publishing it opens the browser to this URL, which is creating a new tag without the v prefix (2 occurrences without v here):

https://github.com/upleveled/eslint-config-upleveled/releases/new?tag=7.8.2&body=-+Switch+module+config+in+...%0A%0Ahttps%3A%2F%2Fgithub.com%2Fupleveled%2Feslint-config-upleveled%2Fcompare%2Fv7.8.1...7.8.2&prerelease=false
karlhorky commented 8 months ago

@mmkal maybe line 11 here?

https://github.com/sindresorhus/np/blob/4b3b599571c9475b2be035b370fef4fe0b2e85d0/source/release-task-helper.js#L1-L18

karlhorky commented 8 months ago

We probably should keep the no-prefix behaviour for pnpm

Hm... my gut feel would be to add prefixes everywhere.

Seems very surprising that if I use pnpm, I suddenly get an unusual, very uncommon tag format for my releases. (I have seen v prefixes pretty much everywhere on GitHub Releases, can't remember a JS/TS ecosystem package that does not use this prefix)

Even pnpm itself uses v prefixes for their GitHub Releases + tags:

mmkal commented 8 months ago

We probably should keep the no-prefix behaviour for pnpm

Hm... my gut feel would be to add prefixes everywhere.

I mean more that we should respect pnpm config get tag-version-prefix - ignoring the config and forcing 'v' would be a breaking change, but I guess one we could make. I'm not sure why pnpm is getting a different result from npm config get, though. Maybe that's a pnpm bug?

karlhorky commented 8 months ago

Yeah, good question why it's returning something different. 🤔

I would lean towards doing the change as a patch release (even if it's technically a breaking change) since the behavior feels like a bug and creates unexpected release names on GitHub.

Maybe @sindresorhus has an opinion.

mmkal commented 8 months ago

Thinking about it more, I now agree. We should just use npm to get the tag prefix since pnpm doesn't ship with a default v. I'm pretty sure that it's what the majority of users would want, and it's still easy enough to do npm config set tag-version-prefix '' if you don't like it.

It still must be a bug that there's a duplicate, but that's a separate, and smaller, problem.

mmkal commented 8 months ago

Opened a PR for it: #739. I also think that this should be considered a bugfix rather than a real breaking change. I will try it out on another package later this week. @karlhorky you should be able to also try it by adding something like "np": "github:sindresorhus/np#pnpm-vs-npm-tag-version-prefix" to your package.json dependencies.

FYI @sindresorhus

karlhorky commented 8 months ago

Nice, I saw that #739 was reviewed and merged and released in np@10.0.2, thanks @mmkal and @sindresorhus !

I just tested np@10.0.2 with a project, and it's working as it was pre-v10 🎉 (no duplicate tags)

From my side, this issue could be closed, but maybe @mmkal you still have identified another issue which is why we should keep it open?

mmkal commented 8 months ago

I suspect we'd have duplicate tags if someone manually set their tag-version-prefix. I haven't tested but let's keep open until someone can.