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

[BUG] regression, npm publish folder has a new and completely incompatible behaviour where folder is interpreted as a package name and is then fetched from the registry #4126

Closed
2 tasks done
cormacrelf opened this issue Dec 6, 2021 · 5 comments
Assignees
Labels
Bug thing that needs fixing Discuss will be discussed at the next internal call Priority 0 will get attention right away Release 8.x work is associated with a specific npm 8 release

Comments

@cormacrelf
Copy link

cormacrelf commented Dec 6, 2021

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

npm publish folder syntax has stopped working almost completely if the folder is not prefixed with a ./ and is not an absolute path. Not only has it stopped working, it is doing utter nonsense, and is producing really weird output as outlined in the repro section below. So much so it no longer appears to be doing anything recognisable as publishing a package at all.

This is a rather critical piece of functionality that is going to break a bunch of scripts all over the internet. I personally found this bug when an unchanged Github Actions workflow stopped publishing canary builds of my package.

The syntax I have used there is npm publish dist --access public --tag canary. The package has been built correctly into a directory named dist, and it has a package.json with a name (that is not dist but @citeproc-rs/wasm). For the past year, this script successfully published dozens of versions of the package, but today it does not. It now fails with an ETARGET message, saying that the package/version dist@canary could not be found. This is completely unexpected, not only because it worked before, but because there is no documented syntax of npm publish where any command line argument accepts a package name. That has always been implied from the package.json!

The Steps to Reproduce below has a guided tour of the insanity that is going on with npm publish.

Expected Behavior

npm publish folder should publish a folder!!! Even if that were not literally the documented syntax of the command, you would expect it to fail with a mysterious error message like ETARGET or a 404. There is no such documented syntax npm publish packagename! Why would it try looking it up on the registry!!!

Steps To Reproduce

  1. mkdir folder
  2. Put a package.json inside it, with "name": "asdfasdfasdf-nonexistent-package"
  3. Observe that the documentation says the syntax is npm publish [<tarball>|<folder>], no mention of folder being a package name
  4. Attempt npm publish folder --dry-run with npm 8.2.0

Watch, in horror, as NPM appears to download the tarball for package folder@0.0.4, list out its contents, and print the summary of the tarball. There is zero relation between doing this and npm publish. I am absolutely speechless.

npm notice
npm notice 📦  folder@0.0.4
npm notice === Tarball Contents ===
npm notice 14B   .npmignore
npm notice 263B  README.md
npm notice 4.9kB lib/html.js
npm notice 2.8kB lib/index.js
npm notice 321B  package.json
...
npm notice === Tarball Details ===
npm notice name:          folder
npm notice version:       0.0.4
npm notice filename:      folder-0.0.4.tgz
npm notice package size:  206.8 kB
npm notice unpacked size: 226.1 kB
npm notice shasum:        56dc666bd686e2f029055e938a003f0f2fadc225
npm notice integrity:     sha512-drevPjCcenZ7O[...]monYcuEYiMK3A==
npm notice total files:   146
npm notice
+ folder@0.0.4

It goes on. The behaviour is different if the name of the directory is not an existing package in the registry.

  1. mv folder asdfasdfasdf-nonexistent-package
  2. npm publish asdfasdfasdf-nonexistent-package --dry-run

Receive:

npm ERR! code E404
npm ERR! 404 Not Found - GET https://registry.npmjs.org/asdfasdfasdf-nonexistent-package - Not found
npm ERR! 404
npm ERR! 404  'asdfasdfasdf-nonexistent-package@latest' is not in this registry.
npm ERR! 404 You should bug the author to publish it (or use the name yourself!)
npm ERR! 404
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, http url, or git url.

There's a THIRD variation. This is when the name of the folder is an existing package on the registry, but you're using the --tag flag, and the package of that name (again, COMPLETELY irrelevant, NPM should not be looking it up at all) does not have that tag. It is relevant to post here because you get more completely different output.

  1. Rename the directory back to folder
  2. npm publish folder --dry-run --tag canary
npm ERR! code ETARGET
npm ERR! notarget No matching version found for folder@canary.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.

There is a workaround: use the syntax npm publish ./folder. Then it knows the folder is a folder.

Environment

  • npm: v8.2.0
  • Node: v17.0.1
  • OS: macOS 12.0.1
  • platform: Macbook Pro
  • npm config:
; "builtin" config from /opt/homebrew/lib/node_modules/npm/npmrc

prefix = "/opt/homebrew"

; "user" config from /Users/me/.npmrc

//registry.npmjs.org/:_authToken = (protected)

; node bin location = /opt/homebrew/Cellar/node/17.0.1/bin/node
; cwd = /Users/me/git/temporary
; HOME = /Users/me
; Run `npm config ls -l` to show all defaults
@cormacrelf cormacrelf added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Dec 6, 2021
@cormacrelf cormacrelf changed the title [BUG] regression, npm publish folder has a new and completely incompatible behaviour where folder is interpreted as a package name [BUG] regression, npm publish folder has a new and completely incompatible behaviour where folder is interpreted as a package name and is then fetched from the registry Dec 6, 2021
@ljharb
Copy link
Collaborator

ljharb commented Dec 6, 2021

The square brackets in the documentation indicates the parameter is optional (a somewhat universal convention) and the pipe indicates either of those two kinds of arguments can be optionally provided. To publish the current folder, you run npm publish with no additional arguments.

I have no explanation or comment on the other behavior you’re reporting.

@cormacrelf
Copy link
Author

cormacrelf commented Dec 6, 2021

Yes, indeed, but if you do in fact provide an argument then it should be interpreted as one of the two things specified in the []. One plausible explanation (but I must again emphasise, very very bad choice) for this behaviour is that it's now [<package>|<tarball>|<./folder>] and if you do select <package> then it attempts to publish PWD as a package with that name, overriding the one in the package.json. That would explain (partially) why it's querying the registry. I was really alarmed when I first saw "146 files" in the folder@0.0.4 tarball summary, I thought it might have uploaded PWD (which had a stack of my other private projects in it).

@cormacrelf
Copy link
Author

Looks like there was recently a large refactoring of all of the CLI front-ends, this is probably a result of that. #3959

@darcyclarke darcyclarke added Priority 0 will get attention right away Discuss will be discussed at the next internal call and removed Needs Triage needs review for next steps labels May 20, 2022
Kapelianovych added a commit to Kapelianovych/edelweiss that referenced this issue Jun 13, 2022
@wraithgar wraithgar self-assigned this Jun 20, 2022
@wraithgar
Copy link
Member

This was changed in npm@7.0.8 as "Support publishing any kind of spec, not just directories"

This allows folks to publish any spec, for instance a tagged git reference: npm publish npm/some-package#v1.2.3. This brings npm publish in line with other commands (such as npm install or npm pack).

The docs will need to be updated to reflect this. If you want to publish a directory you need to prefix the name with ./ so npm knows you are referring to a directory and not another kind of spec.

@wraithgar
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Discuss will be discussed at the next internal call Priority 0 will get attention right away Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

No branches or pull requests

4 participants