Cli: [BUG] invalid package name handling

[richseviora]

What / Why

npm install will silently fail to install most packages if the name in package.json is not valid.

When

• When package name is Keep Local Alive, and I execute npm install.

Where

• Private repo
• Previous discussion: https://npm.community/t/strange-audit-and-install-behavior-when-package-name-is-invalid/89

How

Current Behavior

• Running npm install when the package name is not valid.

Steps to Reproduce

1. Have invalid package name.
2. Run npm install.
3. Look at node_modules/ and see only a few packages.

Expected Behavior

• It should install the packages OR fail loudly.

Who

• n/a

References

• n/a

[sarabpj]

Here is a link to the package name rules. Hope it helps.

[richseviora]

Hi Sara! Just to be clear; I don't have an issue with what the naming rules are, the problem is that npm install silently fails to install all the packages if the name isn't valid.

[DanielRuf]

I think ther emight be some regression

➜  npm-test-1055 npm i react --save
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN Invalid name: "Keep Local Alive"
npm WARN npm-test-1055 No description
npm WARN npm-test-1055 No repository field.
npm WARN npm-test-1055 No README data
npm WARN npm-test-1055 No license field.

+ [email protected]
added 6 packages from 3 contributors and audited 9 packages in 2.608s
found 0 vulnerabilities

➜  npm-test-1055 ll node_modules 
total 0
drwxr-xr-x   7 danielruf  staff   224B 24 Mär 21:32 js-tokens
drwxr-xr-x  10 danielruf  staff   320B 24 Mär 21:32 loose-envify
drwxr-xr-x   6 danielruf  staff   192B 24 Mär 21:32 object-assign
drwxr-xr-x  14 danielruf  staff   448B 24 Mär 21:32 prop-types
drwxr-xr-x   9 danielruf  staff   288B 24 Mär 21:32 react
drwxr-xr-x   9 danielruf  staff   288B 24 Mär 21:32 react-is
➜  npm-test-1055 rm -rf node_modules package-lock.json 
➜  npm-test-1055 npm i
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN Invalid name: "Keep Local Alive"
npm WARN npm-test-1055 No description
npm WARN npm-test-1055 No repository field.
npm WARN npm-test-1055 No README data
npm WARN npm-test-1055 No license field.

up to date in 0.857s
found 0 vulnerabilities

➜  npm-test-1055 ll node_modules
ls: node_modules: No such file or directory

[DanielRuf]

The package-lock.json file contains just this afterwards:

{
  "name": "Keep Local Alive",
  "version": "1.0.0",
  "lockfileVersion": 1
}

[DanielRuf]

I think the only correct solution is to change the package name to a valid one.

[richseviora]

Maybe then just block npm actions if the package name is not valid? I don't think it's worth investing time to fix it, I just don't want it to fail silently.

[ScottG489]

I think this is absolutely worth investing time in to fix. npm install is failing to install yet is exiting with a status code of 0. It's really not acceptable for the INSTALL command to not install yet exit with a successful status code.

It's further confused by the fact that other WARN messages don't result in packages not being installed.

Also as @DanielRuf mentioned, it does seem to be a regression because this used to correctly throw an ERR and not a WARN from research I've done personally.

npm should have the following behavior if the name is invalid:

1. exit with a non-zero status code
2. log at the ERR level rather than WARN

This also seems to be an issue with other fields such as version. Issue #1392 tracks this. There may be other fields as well that are affected.

[ljharb]

It seems to me that an invalid name shouldn't actually interfere with any action but publish or link, since those are the only ones that actually need a valid name. In other words, npm install with an invalid name should always succeed (altho perhaps, it should still warn about the invalid name).

[ljharb]

Can anyone confirm that this issue still occurs in npm 7?

[ScottG489]

@ljharb It doesn't appear to be. Not exactly at least:

```shell script
$ docker run -it --entrypoint sh node:current-alpine3.12
/ # npm --version
7.0.3
/ # mkdir foo && cd foo && npm init --yes
Wrote to /foo/package.json:

{
"name": "foo",
"version": "1.0.0",
"description": "",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
},
"keywords": [],
"author": "",
"license": "ISC"
}

npm notice
npm notice New patch version of npm available! 7.0.3 -> 7.0.5
npm notice Changelog: https://github.com/npm/cli/releases/tag/v7.0.5
npm notice Run npm install -g [email protected] to update!
npm notice
/foo # npm install typescript

added 1 package, and audited 1 package in 1s

found 0 vulnerabilities
/foo # rm -rf node_modules/ package-lock.json
/foo # sed -i 's/foo/foo bar/' package.json
/foo # npm install

added 1 package, and audited 1 package in 941ms

found 0 vulnerabilities
/foo # ls
node_modules package-lock.json package.json
/foo # cat package-lock.json
{
"name": "foo bar",
"version": "1.0.0",
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"name": "foo bar",
"version": "1.0.0",
"license": "ISC",
"dependencies": {
"typescript": "^4.0.3"
}
},
"node_modules/typescript": {
"version": "4.0.3",
"resolved": "https://registry.npmjs.org/typescript/-/typescript-4.0.3.tgz",
"integrity": "sha512-tEu6DGxGgRJPb/mVPIZ48e69xCn2yRmCgYmDugAVwmJ6o+0u1RI18eO7E7WBTLYLaEVVOhwQmcdhQHweux/WPg==",
"bin": {
"tsc": "bin/tsc",
"tsserver": "bin/tsserver"
},
"engines": {
"node": ">=4.2.0"
}
}
},
"dependencies": {
"typescript": {
"version": "4.0.3",
"resolved": "https://registry.npmjs.org/typescript/-/typescript-4.0.3.tgz",
"integrity": "sha512-tEu6DGxGgRJPb/mVPIZ48e69xCn2yRmCgYmDugAVwmJ6o+0u1RI18eO7E7WBTLYLaEVVOhwQmcdhQHweux/WPg=="
}
}
}
```

However, it also doesn't appear to WARN either, and I agree with you that it probably should. Perhaps the requirements on project name validation were relaxed?

Thoughts?

[ljharb]

If it installs with or without a valid name, that suggests the issue is fixed, whether there's a warning or not. Thoughts? @richseviora?

[ScottG489]

I'm happy with that. I think it's debatable if there should be a warning with the install if it could cause other actions to fail. But that can be a separate issue if someone feels the need to make it.

[darcyclarke]

AFAICT, this issue is resolved in 6.14.8 & 7.0.7 (ie. latest 6.x & 7.x). Closing now but feel free to open a new issue using one of our templates if you're still experiencing problems (ref. https://github.com/npm/cli/issues/new/choose).