Enable declaration files in all modules #5
Conversation
…ems that uncovered
| @@ -3,7 +3,7 @@ | |||
| "version": "0.6.0", | |||
| "description": "CDK Constructs for AWS ACM", | |||
| "main": "lib/index.js", | |||
| "types": "lib/index.ts", | |||
| "types": "lib/index.d.ts", | |||
There was a problem hiding this comment.
Can we add a pkglint rule to enforce this? (maybe just open new issue)
There was a problem hiding this comment.
JSII won't compile if types doesn't reference .d.ts file... So you wouldn't even get to pkglint...
| @@ -0,0 +1,8 @@ | |||
| import { Test } from 'nodeunit'; | |||
There was a problem hiding this comment.
Probably should not be checked in....
Missing .gitignore entry somewhere?
There was a problem hiding this comment.
Ah yes, good catch!
| @@ -1,6 +1,6 @@ | |||
| import { App, Stack, StackProps } from 'aws-cdk'; | |||
| import { InlineJavaScriptLambda } from 'aws-cdk-lambda'; | |||
| import { Topic } from '..'; | |||
| import { Topic } from '../lib'; | |||
There was a problem hiding this comment.
If you import { foo } from '..', and it happens .. is the directory where you have package.json, then tsc will read the package.json file and use the types field as the required file. This causes two problems:
- If the
.d.tsfile hasn't been generated yet, compilation will fail onENOENT. - If the
.d.tsfile was produced by a previous compilation, compilation will fail because it becomes part of the source files, andtscrefuses to overwrite a source file.
|
Please confirm that you have a successful dry run. |
|
Addressed commit from @eladb, and I have had a successful dry-run build of the latest changes against |
|
I will leave you to do the squashing and minting of the commit message |
| import { dynamodb } from 'aws-cdk-resources'; | ||
|
|
||
| export { Token }; // Return type of Table#tableName |
There was a problem hiding this comment.
Can't you just fix that?
Also, why wasn't this an error before?
There was a problem hiding this comment.
Wasn't an error before, because tsc didn't have to write a .d.ts file that is coherent using only exported entities. When using the .ts file as the source, the compiler is able to reach any private declaration and make type inference work. I think it's a smell :)
- Upgrade local-npm packages with latest version of JSII (that enables declaration files) - Correct `package.json` files (the `types` field points to the `.d.ts` file now) - Correct `tsconfig.json` files for non-JSII packages, so they enable declaration files. - Add a `.npmignore` file to prevent `npm` from using `.gitignore` instead + Don't pack `.ts` files, but pack `.d.ts` files instead - The change uncovered a bunch of "poor practices", that were fixed as well: + Importing from the package's root (where `package.json` is) cannot work with `.d.ts` files, this was particularly frequent in tests.
|
Merged by 13d8b7f |
- Upgrade local-npm packages with latest version of JSII (that enables declaration files) - Correct `package.json` files (the `types` field points to the `.d.ts` file now) - Correct `tsconfig.json` files for non-JSII packages, so they enable declaration files. - Add a `.npmignore` file to prevent `npm` from using `.gitignore` instead + Don't pack `.ts` files, but pack `.d.ts` files instead - The change uncovered a bunch of "poor practices", that were fixed as well: + Importing from the package's root (where `package.json` is) cannot work with `.d.ts` files, this was particularly frequent in tests.
# This is the 1st commit message: Add Identity Pool construct # This is the commit message #2: Bug fixes # This is the commit message #3: Bug fixes # This is the commit message #4: Formatting # This is the commit message #5: Add construct methods # This is the commit message #6: Remove flat # This is the commit message #7: Fix issues
jsiipackages to the latest build of 0.4.0"declaration": truein the generatedtsconfig.jsonpackage.jsonfiles to reference.d.tsfiles in thetypesfieldtsconfig.jsonfiles to include"declaration": true.npmignorefile to avoidnpmusing.gitignorefor it