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

Consider making @types/* dev dependencies #1326

Open
daniel-white opened this issue Oct 24, 2023 · 6 comments
Open

Consider making @types/* dev dependencies #1326

daniel-white opened this issue Oct 24, 2023 · 6 comments

Comments

@daniel-white
Copy link

It would be great if the @types/* were not direct dependencies but dev dependencies as to avoid the pit fall of having the types for node 20, but running under node 18.

@SBoudrias
Copy link
Owner

SBoudrias commented Oct 26, 2023

This cannot work today because Inquirer code accesses the node types. So it cannot ship without the @types/node as the imported types will be missing. Specifically this caused a bug in the past, see #1241

I'm open to review this or change the code somewhat to get to a better result. But I'll need documentation and reference from you supporting how this is possible. We can work together on that if you're willing to.

@daniel-white
Copy link
Author

it looks like you only ship javascript in your package. so the types package is not directly needed. i would advise using this as a devDependency https://stackoverflow.com/questions/18875674/whats-the-difference-between-dependencies-devdependencies-and-peerdependencie

@SBoudrias
Copy link
Owner

That is not the case, the types are shipped. See https://www.npmjs.com/package/@inquirer/core?activeTab=code under dist/esm/types/ for one example.

@daniel-white
Copy link
Author

then a peerDependency i think might be more appropriate, with the minimum supported version of node

@tuler
Copy link

tuler commented Apr 4, 2024

This also add quite a lot of size to install the package.
2Mb from @types/mute-stream and 2Mb from @types/node.

npx howfat @inquirer/core
@inquirer/core@7.1.1 (28 deps, 2.57mb, 385 files, ©MIT)
├── @inquirer/type@1.2.1 (4.54kb, 7 files, ©MIT)
├─┬ @types/mute-stream@0.0.4 (2 deps, 2.01mb, 105 files, ©MIT)
│ ╰── @types/node@20.12.4 (🔗, 1 dep, 2mb, 100 files, ©MIT)
├─┬ @types/node@20.12.4 (1 dep, 2mb, 100 files, ©MIT)
│ ╰── undici-types@5.26.5 (71.34kb, 35 files, ©MIT)
├── @types/wrap-ansi@3.0.0 (3kb, 4 files, ©MIT)
├─┬ ansi-escapes@4.3.2 (1 dep, 132.26kb, 51 files, ©MIT)
│ ╰── type-fest@0.21.3 (116.26kb, 46 files, ©(MIT OR CC0-1.0))
├─┬ chalk@4.1.2 (5 deps, 95.08kb, 33 files, ©MIT)
│ ├─┬ ansi-styles@4.3.0 (2 deps, 49.67kb, 16 files, ©MIT)
│ │ ╰─┬ color-convert@2.0.1 (1 dep, 33.09kb, 11 files, ©MIT)
│ │   ╰── color-name@1.1.4 (6.54kb, 4 files, ©MIT)
│ ╰─┬ supports-color@7.2.0 (1 dep, 11.19kb, 10 files, ©MIT)
│   ╰── has-flag@4.0.0 (4.32kb, 5 files, ©MIT)
├── cli-spinners@2.9.2 (31.36kb, 6 files, ©MIT)
├── cli-width@4.1.0 (4.66kb, 5 files, ©ISC)
├─┬ figures@3.2.0 (1 dep, 14.42kb, 9 files, ©MIT)
│ ╰── escape-string-regexp@1.0.5 (2.63kb, 4 files, ©MIT)
├── mute-stream@1.0.0 (6.28kb, 4 files, ©ISC)
├── signal-exit@4.1.0 (75.16kb, 29 files, ©ISC)
├─┬ strip-ansi@6.0.1 (1 dep, 9.41kb, 10 files, ©MIT)
│ ╰── ansi-regex@5.0.1 (5.48kb, 5 files, ©MIT)
╰─┬ wrap-ansi@6.2.0 (8 deps, 125.4kb, 48 files, ©MIT)
  ├─┬ ansi-styles@4.3.0 (2 deps, 49.67kb, 16 files, ©MIT)
  │ ╰─┬ color-convert@2.0.1 (1 dep, 33.09kb, 11 files, ©MIT)
  │   ╰── color-name@1.1.4 (6.54kb, 4 files, ©MIT)
  ├─┬ string-width@4.2.3 (4 deps, 66.45kb, 28 files, ©MIT)
  │ ├── emoji-regex@8.0.0 (47.12kb, 8 files, ©MIT)
  │ ├── is-fullwidth-code-point@3.0.0 (4.88kb, 5 files, ©MIT)
  │ ╰── strip-ansi@6.0.1 (🔗, 1 dep, 9.41kb, 10 files, ©MIT)
  ╰── strip-ansi@6.0.1 (🔗, 1 dep, 9.41kb, 10 files, ©MIT)

@SBoudrias
Copy link
Owner

Like stated earlier, I'm happy to review this. But we need a solution that'll also work for typescript users.

dependencies appears to be how we need to handle this for now; see here. And you should also refer to the bug it caused in #1241.

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

No branches or pull requests

3 participants