Skip to content

Commit 2704008

Browse files
authored
chore: require type imports in published packages to be from declared dependencies (#472)
When we import a type, this import may become part of the public declarations of a package. Previously we allowed types to be imported from devDeps. This can cause consuming TypeScript packages to fail compilation if they have `skipLibCheck: false` set (which is the default). To resolve this, we are now enforcing all type imports to be from a declared dependency. The recommendation is to declare a very wide peer dependency, if the package is a types only package. Note that not all imports of types will actually end up in the published declaration files. This only happens if the imported type is part of an exported API for that file, e.g. part of a function signature. Unfortunately there is no good way of checking just this case with eslint, so we do the next best thing. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 979793d commit 2704008

File tree

13 files changed

+90
-51
lines changed

13 files changed

+90
-51
lines changed

.projenrc.ts

Lines changed: 15 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { AdcPublishing } from './projenrc/adc-publishing';
77
import { BundleCli } from './projenrc/bundle';
88
import { CdkCliIntegTestsWorkflow } from './projenrc/cdk-cli-integ-tests';
99
import { CodeCovWorkflow } from './projenrc/codecov';
10-
import { ESLINT_RULES } from './projenrc/eslint';
10+
import { configureEslint } from './projenrc/eslint';
1111
import { IssueLabeler } from './projenrc/issue-labeler';
1212
import { JsiiBuild } from './projenrc/jsii';
1313
import { LargePrChecker } from './projenrc/large-pr-checker';
@@ -21,54 +21,32 @@ import { TypecheckTests } from './projenrc/TypecheckTests';
2121
const TYPESCRIPT_VERSION = '5.6';
2222

2323
/**
24-
* Projen depends on TypeScript-eslint 7 by default.
25-
*
26-
* We want 8 for the parser, and 6 for the plugin (because after 6 some linter
27-
* rules we are relying on have been moved to another plugin).
28-
*
29-
* Also configure eslint plugins & rules, which cannot be configured by props.
24+
* Configures a Eslint, which is a complex setup.
3025
*
3126
* We also need to override the built-in prettier dependency to prettier@2, because
3227
* Jest < 30 can only work with prettier 2 (https://github.com/jestjs/jest/issues/14305)
3328
* and 30 is not stable yet.
3429
*/
3530
function configureProject<A extends pj.typescript.TypeScriptProject>(x: A): A {
31+
// currently supported min node version
3632
x.package.addEngine('node', '>= 14.15.0');
33+
3734
x.addDevDeps(
38-
'@typescript-eslint/eslint-plugin@^8',
39-
'@typescript-eslint/parser@^8',
40-
'@stylistic/eslint-plugin@^3',
41-
'@cdklabs/eslint-plugin',
42-
'eslint-plugin-import',
43-
'eslint-plugin-jest',
44-
'eslint-plugin-jsdoc',
4535
'jest-junit@^16',
36+
'prettier@^2.8',
4637
);
47-
x.eslint?.addPlugins(
48-
'@typescript-eslint',
49-
'import',
50-
'@cdklabs',
51-
'@stylistic',
52-
'jest',
53-
'jsdoc',
54-
);
55-
x.eslint?.addExtends(
56-
'plugin:jest/recommended',
57-
);
58-
x.eslint?.addIgnorePattern('*.generated.ts');
59-
x.eslint?.addRules(ESLINT_RULES);
60-
61-
// Prettier needs to be turned off for now, there's too much code that doesn't conform to it
62-
x.eslint?.addRules({ 'prettier/prettier': ['off'] });
6338

64-
x.addDevDeps('prettier@^2.8');
39+
configureEslint(x);
6540

66-
x.npmignore?.addPatterns('.eslintrc.js');
67-
// As a rule we don't include .ts sources in the NPM package
68-
x.npmignore?.addPatterns('*.ts', '!*.d.ts');
69-
70-
// Never include the build-tools directory
71-
x.npmignore?.addPatterns('build-tools');
41+
x.npmignore?.addPatterns(
42+
// don't inlcude config files
43+
'.eslintrc.js',
44+
// As a rule we don't include .ts sources in the NPM package
45+
'*.ts',
46+
'!*.d.ts',
47+
// Never include the build-tools directory
48+
'build-tools',
49+
);
7250

7351
if (x instanceof TypeScriptWorkspace) {
7452
// Individual workspace packages shouldn't depend on "projen", it gets brought in at the monorepo root

packages/@aws-cdk-testing/cli-integ/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/cli-lib-alpha/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/cli-plugin-contract/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/cloud-assembly-schema/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/cloudformation-diff/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/integ-runner/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk/toolkit-lib/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/aws-cdk/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/aws-cdk/lib/commands/migrate.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
/* eslint-disable @typescript-eslint/no-var-requires */
33
import * as fs from 'fs';
44
import * as path from 'path';
5-
import type { ForReading } from '@aws-cdk/cli-plugin-contract';
65
import type { Environment } from '@aws-cdk/cx-api';
76
import { UNKNOWN_ACCOUNT, UNKNOWN_REGION } from '@aws-cdk/cx-api';
87
import type {
@@ -24,6 +23,7 @@ import { ToolkitError } from '../../../@aws-cdk/toolkit-lib';
2423
import { info } from '../../lib/logging';
2524
import type { ICloudFormationClient, SdkProvider } from '../api/aws-auth';
2625
import { CloudFormationStack } from '../api/cloudformation';
26+
import { Mode } from '../api/plugin';
2727
import { zipDirectory } from '../util';
2828
const camelCase = require('camelcase');
2929
const decamelize = require('decamelize');
@@ -143,7 +143,7 @@ export async function readFromStack(
143143
sdkProvider: SdkProvider,
144144
environment: Environment,
145145
): Promise<string | undefined> {
146-
const cloudFormation = (await sdkProvider.forEnvironment(environment, 0 satisfies ForReading)).sdk.cloudFormation();
146+
const cloudFormation = (await sdkProvider.forEnvironment(environment, Mode.ForReading)).sdk.cloudFormation();
147147

148148
const stack = await CloudFormationStack.lookup(cloudFormation, stackName, true);
149149
if (stack.stackStatus.isDeploySuccess || stack.stackStatus.isRollbackSuccess) {
@@ -605,7 +605,7 @@ export function buildGenertedTemplateOutput(
605605
* @returns A CloudFormation sdk client
606606
*/
607607
export async function buildCfnClient(sdkProvider: SdkProvider, environment: Environment) {
608-
const sdk = (await sdkProvider.forEnvironment(environment, 0 satisfies ForReading)).sdk;
608+
const sdk = (await sdkProvider.forEnvironment(environment, Mode.ForReading)).sdk;
609609
sdk.appendCustomUserAgent('cdk-migrate');
610610
return sdk.cloudFormation();
611611
}

packages/cdk-assets/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/cdk/.eslintrc.json

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

projenrc/eslint/index.ts

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { yarn } from 'cdklabs-projen-project-types';
2+
import type { typescript } from 'projen';
13
import bestPractices from './best-practices';
24
import constructs from './constructs';
35
import formatting from './formatting';
@@ -6,12 +8,61 @@ import jest from './jest';
68
import jsdoc from './jsdoc';
79
import team from './team';
810

9-
export const ESLINT_RULES = {
11+
const ESLINT_RULES = {
1012
...team,
1113
...bestPractices,
1214
...constructs,
1315
...imports,
1416
...formatting,
1517
...jsdoc,
1618
...jest,
19+
20+
// Prettier needs to be turned off for now, there's too much code that doesn't conform to it
21+
'prettier/prettier': ['off'],
1722
};
23+
24+
/**
25+
* Projen depends on TypeScript-eslint 7 by default.
26+
*
27+
* We want 8 for the parser, and 6 for the plugin (because after 6 some linter
28+
* rules we are relying on have been moved to another plugin).
29+
*
30+
* Also configure eslint plugins & rules, which cannot be configured by props.
31+
*/
32+
export function configureEslint(x: typescript.TypeScriptProject) {
33+
const isRoot = x instanceof yarn.Monorepo;
34+
const isPrivate = x instanceof yarn.TypeScriptWorkspace && x.isPrivatePackage;
35+
36+
// configure deps and plugins
37+
x.addDevDeps(
38+
'@typescript-eslint/eslint-plugin@^8',
39+
'@typescript-eslint/parser@^8',
40+
'@stylistic/eslint-plugin@^3',
41+
'@cdklabs/eslint-plugin',
42+
'eslint-plugin-import',
43+
'eslint-plugin-jest',
44+
'eslint-plugin-jsdoc',
45+
);
46+
x.eslint?.addPlugins(
47+
'@typescript-eslint',
48+
'import',
49+
'@cdklabs',
50+
'@stylistic',
51+
'jest',
52+
'jsdoc',
53+
);
54+
55+
// ignore files
56+
x.eslint?.addIgnorePattern('*.generated.ts');
57+
58+
// base rules from plugins and our sets
59+
x.eslint?.addExtends(
60+
'plugin:jest/recommended',
61+
);
62+
x.eslint?.addRules(ESLINT_RULES);
63+
64+
// For our published packages, we need all type imports to be from a public dependency
65+
if (!isRoot && !isPrivate && x.eslint) {
66+
x.eslint.rules['import/no-extraneous-dependencies'][1].includeTypes = true;
67+
}
68+
}

0 commit comments

Comments
 (0)