From a6358630abaaba33ffaf2bc638e168a722678b31 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 22 Feb 2023 10:43:55 +1030 Subject: [PATCH] review fixes --- .../util/parsers/typescript-estree-import.ts | 1 - .../eslint-plugin-tslint/tsconfig.build.json | 1 + packages/eslint-plugin/rules.d.ts | 32 +++++++++++++++++-- packages/eslint-plugin/tsconfig.build.json | 1 + patches/eslint+8.34.0.patch | 15 +++++++++ yarn.lock | 2 +- 6 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 patches/eslint+8.34.0.patch diff --git a/packages/ast-spec/tests/util/parsers/typescript-estree-import.ts b/packages/ast-spec/tests/util/parsers/typescript-estree-import.ts index e4efe29db09..7474f4eeecc 100644 --- a/packages/ast-spec/tests/util/parsers/typescript-estree-import.ts +++ b/packages/ast-spec/tests/util/parsers/typescript-estree-import.ts @@ -13,5 +13,4 @@ * This should be the only place in the package that we import from typescript-estree. */ -// We need to ignore this lint error regarding it being missing from the package.json, see above. export { parse } from '@typescript-eslint/typescript-estree'; diff --git a/packages/eslint-plugin-tslint/tsconfig.build.json b/packages/eslint-plugin-tslint/tsconfig.build.json index 1eb8c0821e2..86650784975 100644 --- a/packages/eslint-plugin-tslint/tsconfig.build.json +++ b/packages/eslint-plugin-tslint/tsconfig.build.json @@ -2,6 +2,7 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { // specifically disable declarations for the plugin + // see reasoning in packages/eslint-plugin/rules.d.ts "declaration": false, "declarationMap": false, "outDir": "./dist", diff --git a/packages/eslint-plugin/rules.d.ts b/packages/eslint-plugin/rules.d.ts index 0b154ba78d6..9a5272d205c 100644 --- a/packages/eslint-plugin/rules.d.ts +++ b/packages/eslint-plugin/rules.d.ts @@ -1,10 +1,38 @@ /* -We purposely don't generate types for our plugin because: +We purposely don't generate types for our plugin because TL;DR: 1) there's no real reason that anyone should do a typed import of our rules, 2) it would require us to change our code so there aren't as many inferred types This type declaration exists as a hacky way to add a type to the export for our -internal packages that require it +internal packages that require it. + +*** Long reason *** + +When you turn on declaration files, TS requires all types to be "fully resolvable" +without changes to the code. +All of our lint rules `export default createRule(...)`, which means they all +implicitly reference the `TSESLint.Rule` type for the export. + +TS wants to transpile each rule file to this `.d.ts` file: + +```ts +import type { TSESLint } from '@typescript-eslint/utils'; +declare const _default: TSESLint.RuleModule; +export default _default; +``` + +Because we don't import `TSESLint` in most files, it means that TS would have to +insert a new import during the declaration emit to make this work. +However TS wants to avoid adding new imports to the file because a new module +could have type side-effects (like global augmentation) which could cause weird +type side-effects in the decl file that wouldn't exist in source TS file. + +So TS errors on most of our rules with the following error: +``` +The inferred type of 'default' cannot be named without a reference to +'../../../../node_modules/@typescript-eslint/utils/src/ts-eslint/Rule'. +This is likely not portable. A type annotation is necessary. ts(2742) +``` */ import type { RuleModule } from '@typescript-eslint/utils/ts-eslint'; diff --git a/packages/eslint-plugin/tsconfig.build.json b/packages/eslint-plugin/tsconfig.build.json index af60c77e84f..49f58646073 100644 --- a/packages/eslint-plugin/tsconfig.build.json +++ b/packages/eslint-plugin/tsconfig.build.json @@ -2,6 +2,7 @@ "extends": "../../tsconfig.base.json", "compilerOptions": { // specifically disable declarations for the plugin + // see reasoning in packages/eslint-plugin/rules.d.ts "declaration": false, "declarationMap": false, "outDir": "./dist", diff --git a/patches/eslint+8.34.0.patch b/patches/eslint+8.34.0.patch new file mode 100644 index 00000000000..d9302432437 --- /dev/null +++ b/patches/eslint+8.34.0.patch @@ -0,0 +1,15 @@ +diff --git a/node_modules/eslint/lib/rules/indent.js b/node_modules/eslint/lib/rules/indent.js +index cda0203..66714b4 100644 +--- a/node_modules/eslint/lib/rules/indent.js ++++ b/node_modules/eslint/lib/rules/indent.js +@@ -12,7 +12,9 @@ + // Requirements + //------------------------------------------------------------------------------ + +-const { OrderedMap } = require("js-sdsl"); ++// make this a deep import so that rollup doesn't attempt to bundle the downleveled ESM build ++// https://github.com/js-sdsl/js-sdsl/issues/158 ++const { default: OrderedMap } = require("js-sdsl/dist/cjs/container/TreeContainer/OrderedMap"); + + const astUtils = require("./utils/ast-utils"); + diff --git a/yarn.lock b/yarn.lock index a21ab2b4af6..54d2b8cdeab 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9687,7 +9687,7 @@ lzstring.ts@^2.0.2: dependencies: tslib "^1.10.0" -magic-string@^0.25.0, magic-string@^0.25.7, magic-string@^0.25.9: +magic-string@0.25.9, magic-string@^0.25.0, magic-string@^0.25.7: version "0.25.9" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.9.tgz#de7f9faf91ef8a1c91d02c2e5314c8277dbcdd1c" integrity sha512-RmF0AsMzgt25qzqqLc1+MbHmhdx0ojF2Fvs4XnOqz2ZOBXzzkEwc/dJQZCYHAn7v1jbVOjAZfK8msRn4BxO4VQ==