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

doc: Mention possibility of corepack enable npm #53257

Closed
wants to merge 3 commits into from

Conversation

JonnyBurger
Copy link

@JonnyBurger JonnyBurger commented Jun 2, 2024

After reading https://www.totaltypescript.com/how-to-use-corepack, I realized that the Corepack docs page has some shortfalls:

  • It implies npm is not supported, but npm is supported. It is just not enabled by default.
  • The command for enabling the npm shim (corepack enable npm) is never mentioned anywhere.

With this PR, I believe to correct the docs according to the current implementation. If you anyways plan to change it, feel free to close.

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Jun 2, 2024
doc/api/corepack.md Outdated Show resolved Hide resolved
| [pnpm][] | `pnpm`, `pnpx` |
| [npm][]* | `npm`, `npx` |

\* Not enabled by default. Use `corepack enable npm` to do so.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is only referring to NPM, couldn't it be referenced by name, and not a "*"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do that if you want me to.
Wanted to raise attention that it is a special case as soon as you read the entry in the table.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s your call, I’m not a CODEOWNER, so it’s up to you :-).

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion but there is quite a lot of history here, the npm team does not support and discourages this (though they believe it's not their place to tell us).

cc @nodejs/package-maintenance

@wesleytodd
Copy link
Member

Due to the situation here I believe this change cannot land. See nodejs/corepack#418 for more details.

@JonnyBurger
Copy link
Author

Understood the situation is complicated.
I'm not pushing for it to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants