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

About optionalDependencies on package.json #3486

Closed
alexojegu opened this issue Dec 15, 2023 · 7 comments · Fixed by #3493
Closed

About optionalDependencies on package.json #3486

alexojegu opened this issue Dec 15, 2023 · 7 comments · Fixed by #3493
Assignees
Labels
question Further information is requested

Comments

@alexojegu
Copy link
Contributor

It is really necessary to have packages like eslint, mocha, c8, etc. as optionalDependencies. If I'm not ignoring something, it seems that it would be more correct for them to be devDependencies.

I say this because when installing zwave-js-ui as an NPM package it installs 1172 packages and among them I can see for example:

# ls -l /usr/local/lib/node_modules/zwave-js-ui/node_modules/ | grep eslint
drwxr-xr-x    1 root     root            20 Dec 15 21:11 @eslint
drwxr-xr-x    1 root     root            38 Dec 15 21:11 @eslint-community
drwxr-xr-x    1 root     root           256 Dec 15 21:11 conventional-changelog-eslint
drwxr-xr-x    1 root     root           116 Dec 15 21:12 eslint
drwxr-xr-x    1 root     root            96 Dec 15 21:11 eslint-import-resolver-node
drwxr-xr-x    1 root     root           394 Dec 15 21:11 eslint-module-utils
drwxr-xr-x    1 root     root           114 Dec 15 21:11 eslint-plugin-babel
drwxr-xr-x    1 root     root            62 Dec 15 21:11 eslint-plugin-es
drwxr-xr-x    1 root     root           224 Dec 15 21:12 eslint-plugin-import
drwxr-xr-x    1 root     root            86 Dec 15 21:11 eslint-plugin-node
drwxr-xr-x    1 root     root           112 Dec 15 21:11 eslint-plugin-promise
drwxr-xr-x    1 root     root            62 Dec 15 21:12 eslint-plugin-vue
drwxr-xr-x    1 root     root            92 Dec 15 21:11 eslint-rule-composer
drwxr-xr-x    1 root     root            70 Dec 15 21:11 eslint-scope
drwxr-xr-x    1 root     root           164 Dec 15 21:11 eslint-utils
drwxr-xr-x    1 root     root            70 Dec 15 21:11 eslint-visitor-keys
drwxr-xr-x    1 root     root           116 Dec 15 21:11 vue-eslint-parser

# npm -g ls eslint
/usr/local/lib
└─┬ zwave-js-ui@9.6.0
  ├─┬ eslint-plugin-babel@5.3.1
  │ └── eslint@8.55.0 deduped
  ├─┬ eslint-plugin-import@2.29.1
  │ └── eslint@8.55.0 deduped
  ├─┬ eslint-plugin-node@11.1.0
  │ ├─┬ eslint-plugin-es@3.0.1
  │ │ └── eslint@8.55.0 deduped
  │ └── eslint@8.55.0 deduped
  ├─┬ eslint-plugin-promise@6.1.1
  │ └── eslint@8.55.0 deduped
  ├─┬ eslint-plugin-vue@9.19.2
  │ ├─┬ @eslint-community/eslint-utils@4.4.0
  │ │ └── eslint@8.55.0 deduped
  │ ├── eslint@8.55.0 deduped
  │ └─┬ vue-eslint-parser@9.3.2
  │   └── eslint@8.55.0 deduped
  └── eslint@8.55.0
@alexojegu alexojegu added the question Further information is requested label Dec 15, 2023
@robertsLando
Copy link
Member

Them are there just because:

The term optional dependencies apply to dependencies that won't cause a failure during the installation of an application or project since npm will ignore them if they fail. Whether these dependencies are present or not, the application will still work as expected

@alexojegu
Copy link
Contributor Author

I suppose it will be useful for CI, for the rest of the cases I don't quite see the usefulness because those dependencies for production do not seem to be necessary and for development they seem highly recommended (although not required).

I do not know why:

npm install -g --omit=optional zwave-js-ui

Installs the same 1172 dependencies and doesn't skip the optional ones.

@robertsLando
Copy link
Member

Could be an issue with your npm version? What is your npm/nodejs version?

@alexojegu
Copy link
Contributor Author

I was previously using nodejs 18, this last weekend I upgraded it to nodejs 20 and retry to install zwave-js-ui with same result.

  • nodejs: v20.10.0
  • npm: 10.2.5

@robertsLando
Copy link
Member

I may be missing something here but optional dependencies should be skipped, I will look at this tomorrow, thanks for letting me know 🙏🏻

@alexojegu
Copy link
Contributor Author

Great, now it install 445 packages vs previous 1172 packages.

@robertsLando
Copy link
Member

@alexojegu thanks for your feedback Alex glad the issue is now solved 🙌🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants