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

Include @types/ dependencies in dependencies not devDependencies #2895

Closed
Labels
bug This issue is a bug.

Comments

@everett1992
Copy link
Contributor

Describe the bug
Typescript packages using aws-cdk cannot find a declaration file for module 'yargs'
To Reproduce

Create a typescript package that requires aws-cdk.

mkdir repro && cd repro
echo "{}" > package.json
npm install [email protected] [email protected]
./node_modules/.bin/tsc --init
echo "import 'aws-cdk'" > index.ts
./node_modules/.bin/tsc
node_modules/aws-cdk/lib/settings.d.ts:1:24 - error TS7016: Could not find a declaration file for module 'yargs'. '/local/home/calebev/repro/node_modules/yargs/index.js' implicitly has an 'any' type.
  Try `npm install @types/yargs` if it exists or add a new declaration (.d.ts) file containing `declare module 'yargs';`

Typescript will type check libraries by default. It will fail checking aws-cdk because cdk does not include it's dependencies types in dependencies, it includes them in devDependencies.

Customers can fix this issue by

  • adding @types/yargs to their dependencies.
  • adding --skipLibCheck to their typescript options.

Expected behavior
There should not be any type errors in an effectively empty package. aws-cdk should include it's type dependencies in it's dependencies.

Version:

  • linux
  • javascript
  • 0.34.0
@everett1992 everett1992 added the bug This issue is a bug. label Jun 17, 2019
@RomainMuller
Copy link
Contributor

I think in fact the real problem is that we should not be exporting symbols that depend on a particular 3P dependency. There is no actual need for they args declarations there (we can live with a much simpler, much more restrictive type here).

RomainMuller added a commit that referenced this issue Jun 18, 2019
The settings.d.ts file was referencing `yargs`, but in fact only using the
`yargs.Arguments` type as a stand-in for a `{ [key: string]: unknown }` alias.
Replaced with the more restrictive type so we do not have an exported
dependency on `yargs` types.

Fixes #2895
RomainMuller added a commit that referenced this issue Jun 18, 2019
The settings.d.ts file was referencing `yargs`, but in fact only using the
`yargs.Arguments` type as a stand-in for a `{ [key: string]: unknown }` alias.
Replaced with the more restrictive type so we do not have an exported
dependency on `yargs` types.

Fixes #2895
This was referenced Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment