-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
fix(docs): clarify what install type gets .bins #3491
Conversation
TBH, even knowing the distinction you’re trying to make from comments you left in #3488 this change wouldn’t make that clear to me. |
“On install” vs “when this package is installed” read the same to me, as someone just trying to is that wording meaningful to more experienced npm devs? |
The fact that some people have a mental model of "running What wording would you recommend then? |
I’m not at a computer at the moment. But distinguishing those two cases by path might be more clear. roughly: When installing to $globalDir, this script’s bin gets installed. When installing to node_modules/.bin, this script’s dependencies’ “bin” get installed. (and maybe also a note about the thing you mentioned — that it’s not recommended to directly use nod_modules/.bin scripts directly.) |
I think mentioning the
|
Ok that's done. Thanks for helping make sure the docs make sense to you. |
installs. | ||
command name to local file name. When this packaged is installed, npm | ||
will symlink that file into a place where it can be invoked by name if | ||
installed globally, or by `npm exec` when installed in another package. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
installed globally, or by `npm exec` when installed in another package. | |
installed globally, or by `npm exec` or `npm run-script` when installed in another package. |
“when installed globally” is only true when the PATH contains the global npm bin location; should that be called out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "source" of truth for executables is at https://docs.npmjs.com/cli/v7/configuring-npm/folders#executables so we should link there instead of trying to re-describe it.
Documenting "{prefix}/bin needs to be in your PATH" should probably be in the configuring section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually run-script
is incorrect. That only works for scripts
entries, not bin
. Removing that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm run-script
entries in "another package" definitely has access to the bin tho; that's what i was trying to convey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yeah, you can't npm run-script
that package by name but you can refer to it by name in things run via npm run-script
. I hope the new copy reflects this better.
614672b
to
e4250e0
Compare
Copy has been updated to clarify even more how the bins are intended to be used locally or globally, and a note on adding your global bin to |
e4250e0
to
e557c78
Compare
Thanks @ljharb. We were too late for today's release so this'll go out in two weeks. |
"on install" was ambiguous because it wasn't clear if it meant "when npm install is ran on this project" or "when this project is installed somewhere else" PR-URL: #3491 Credit: @wraithgar Close: #3491 Reviewed-by: @ljharb
e557c78
to
339145f
Compare
"on install" was ambiguous because it wasn't clear if it meant "when npm
install is ran on this project" or "when this project is installed
somewhere else"
References
Related to #3488