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] npm publish complains on bin field examples from npm docs #7302

Closed
2 tasks done
mrazauskas opened this issue Mar 20, 2024 · 12 comments
Closed
2 tasks done

[BUG] npm publish complains on bin field examples from npm docs #7302

mrazauskas opened this issue Mar 20, 2024 · 12 comments
Assignees
Labels
Bug thing that needs fixing Documentation documentation related issue Priority 2 secondary priority issue Release 10.x

Comments

@mrazauskas
Copy link

mrazauskas commented Mar 20, 2024

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 complains and auto correct the bin field examples from npm docs.

Expected Behavior

Seems like the examples are correct, there is no need to complain or alter the code.

Steps To Reproduce

First example:

  1. Create fresh repo with npm init -y
  2. Add dummy file cli.js
  3. Copy bin field entry from npm docs to package.json:
"bin": {
  "myapp": "./cli.js"
}

Second example:

  1. Create fresh repo with npm init -y
  2. Add dummy file cli.js
  3. Copy bin field entry from npm docs to package.json:
"bin": "./cli.js"

Finally run npm publish --dry-run. In both cases npm complains:

npm WARN publish npm auto-corrected some errors in your package.json when publishing.  Please run "npm pkg fix" to address these errors.
npm WARN publish errors corrected:
npm WARN publish "bin[myapp]" script name was cleaned

Environment

  • npm: 10.2.4
  • Node.js: 20.11.1
  • OS Name: macOS
  • System Model Name: Macbook Pro
  • npm config:
; "user" config from /Users/(replaced)/.npmrc

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

; node bin location = /Users/(replaced)/.n/bin/node
; node version = v20.11.1
; npm local prefix = /Users/(replaced)/Projects/x-npm-bin
; npm version = 10.2.4
; cwd = /Users/(replaced)/Projects/x-npm-bin
; HOME = /Users/(replaced)
; Run `npm config ls -l` to show all defaults.
@mrazauskas mrazauskas added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Mar 20, 2024
@milaninfy milaninfy added Documentation documentation related issue Priority 2 secondary priority issue and removed Needs Triage needs review for next steps labels Mar 21, 2024
@trivikr
Copy link

trivikr commented Mar 26, 2024

Related: npm/package-json#83

@Santoshraj2 Santoshraj2 self-assigned this May 9, 2024
@Santoshraj2
Copy link
Contributor

@mrazauskas As we are trying to create the executable files in the bin folder , the executable files should be in bin folder and path should be the directory to the bin folder . if we keep it out of bin and publish it , It will give us the same warning as it is a required to put the files in bin and same path should be given in package.json file.

do below steps and it should work as expected.

  1. create cli.js file inside bin folder
  2. make below entry in package.json file.
    "bin": {
    "myapp": "bin/cli.js"
    }

@mrazauskas
Copy link
Author

Thanks for comment. It works, of course. I just try to explain that npm complains about the examples from its own documentation website.

@mrazauskas
Copy link
Author

@Santoshraj2 Hm.. I was speaking about the two examples just under the one you have fixed. Also not sure if these examples is the problem. As far as I can understand ./path/to/program and path/to/program are equivalent. Might be I mis something, but it also can be that validation logic of the bin field is incorrect.

@Santoshraj2
Copy link
Contributor

@mrazauskas Yes you are right ./path/to/program and path/to/program are same. so other examples looks fine to me.

@mrazauskas
Copy link
Author

Yes you are right ./path/to/program and path/to/program are same. so other examples looks fine to me.

@Santoshraj2 Exactly! The examples look right to me too, but npm publish is giving a warning. And that is what this issue was about! The problem was not fixed, but unfortunately the issue got closed.

@Santoshraj2
Copy link
Contributor

@Santoshraj2 Hm.. I was speaking about the two examples just under the one you have fixed. Also not sure if these examples is the problem. As far as I can understand ./path/to/program and path/to/program are equivalent. Might be I mis something, but it also can be that validation logic of the bin field is incorrect.

The examples which you are talking about is just representing two ways of mentioning executable files. when you have one you can say bin : "path" and for multiple its bin:{ }.. this never says you to keep executables in root folder. i am saying ./bin/cli.js is similar to bin/cli.js.

@mrazauskas
Copy link
Author

These exaples give a warning. Just try them. You will see a warning. That is the issue.

If examples are not correct, they must be fixed.

If warning is not correct, it must be not emitted.

If there is some other problem, it must be fixed.

Currently this issue is closed, but it is not fixed. Looks like nobody really followed the steps I described.

@mrazauskas
Copy link
Author

@Santoshraj2 Please read the description of the issue and follow the steps provided there. Do you see warnings emitted?

@Santoshraj2
Copy link
Contributor

@Santoshraj2 Please read the description of the issue and follow the steps provided there. Do you see warnings emitted?

I clearly followed the steps, thats why we agreed that its a problem and required doc fix.

But there are 2 sides of problem:
1.
keeping cli.js in root folder and then modifying package.json as "bin": {
"myapp": "./cli.js"
}

In this case we got warning,

but when we keep cli.js inside bin folder with package.json as "bin": {
"myapp": "bin/cli.js"
}
its not giving warning and working fine, so we concluded its a document which needs to be modified.
we raised PR and closed it.

  1. keeping cli.js in bin folder and package.json has "bin": {
    "myapp": "bin/cli.js"
    }

its not giving warning and working fine

keeping cli.js in bin folder and package.json has "bin": {
"myapp": "./bin/cli.js"
}
its giving warning, since ./bin/cli.js and bin/cli.js is similar , so this will not require document change but i believe it will be a code fix that warning should not appear.. we will confirm on this and will update you.

@wraithgar
Copy link
Member

The warning is there to let folks know of the changes npm has always been making to the package.json during publish. The fact that it is surprising to folks is exactly why that warning is there. The package.json in your package and the manifest in the registry differ, sometimes by a lot. This is not ideal, and we are working towards limiting those differences. The beginning of that work was to log a warning whenever what was published was different than what was on disk.

So while it may seem like "./foo" and "foo" are similar, they are not. They never have been as far as npm's directory parsing is concerned. Remember we are dealing with 12 years of built up history, and not often built from a spec. If you run npm pkg fix and still get warnings, THAT is worth a bug report. If the docs suggest something that npm pkg fix will correct, that is worth a documentation fix.

wraithgar pushed a commit that referenced this issue May 14, 2024
Our existing example present in npm doc was giving warning.

issue: #7302
@mrazauskas
Copy link
Author

Thanks a lot for detailed explanation. I do understand this is only a minor detail, but it felt important to point out that there is a difference between actual behaviour and documentation.

Apologies if my comments sounded repetitive or explanations were hard to understand. Now I get that this is a documentation issue only. Glad to see it being fixed. Thanks again for taking care.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Documentation documentation related issue Priority 2 secondary priority issue Release 10.x
Projects
None yet
Development

No branches or pull requests

5 participants