Skip to content

Warn on unknown configs #8071

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

Merged
merged 5 commits into from
Jan 30, 2025
Merged

Warn on unknown configs #8071

merged 5 commits into from
Jan 30, 2025

Conversation

wraithgar
Copy link
Member

@wraithgar wraithgar commented Jan 28, 2025

This PR will:

  • Make npm log a warning when it is parsing a config item that is not defined.
  • Make npm log a warning when it is expanding abbreviated config items.
  • Warn when an unknown cli flag is making what the user may have intended as a config value be parsed as a positional arg.
  • Move nerf-dart configs that were originally hard coded in the npm config command into @npmcli/config for reuse.
  • Update nopt to 8.1.0 which has the new features needed to implement these new warnings for cli flags.

terminal output showing new warnings in effect
terminal output showing how to use -- to isolate npm args from script args in npm exec

It is not a valid cli flag, single-hyphen flags should all be single-character.  Eventually `-ws` will need to go away so will at least stop suggesting it now.
@wraithgar wraithgar requested a review from a team as a code owner January 28, 2025 19:09
Copy link

@Tayvon Tayvon left a comment

Choose a reason for hiding this comment

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

LGTM!

@ljharb
Copy link
Contributor

ljharb commented Jan 30, 2025

Can there be a config to silence these warnings?

@wraithgar
Copy link
Member Author

No there will not. This is an important warning that npm is doing something unexpected. The only place this will likely remain a permanent warning and not an eventual exception is publishConfig because that is ostensibly something other package managers may use. cli flags and npmrc files are not for parsing by things that aren't npm. The warnings from publishConfig will likely change to a "this is being ignored" message when we move to fully disallowing undefined configs, but it will always be there. npm has no way of knowing what in there is legitimate or a mistake so it will always warn.

@wraithgar wraithgar merged commit ed85b01 into latest Jan 30, 2025
46 checks passed
@wraithgar wraithgar deleted the gar/invalid-config branch January 30, 2025 16:45
@github-actions github-actions bot mentioned this pull request Jan 30, 2025
@ljharb
Copy link
Contributor

ljharb commented Jan 30, 2025

I agree that anything in npmrc or cli flags is npm's sole purview, and warnings make sense - but things in package.json aren't and they often won't.

wraithgar added a commit that referenced this pull request Mar 6, 2025
pr #8071 originally had this but that appears to have gotten lost along the way.
wraithgar added a commit that referenced this pull request Mar 7, 2025
PR #8071 originally had this but that appears to have gotten lost along
the way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants