From f1e26b8ed7fcf34a813a959acc06e1ff04f844ff Mon Sep 17 00:00:00 2001 From: Marat Dyatko Date: Thu, 23 Apr 2026 16:28:20 +0200 Subject: [PATCH] Address review feedback on no-self-package-imports rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `visitImports` now also reads `exportKind` so `export type { … } from` statements are classified as type-only, fixing a false positive in the self-import rule (and correctly skipping them in `no-undeclared-imports` too). - The reachability-graph regex in `no-self-package-imports` skips `import type` / `export type` edges so files reachable only via type-only re-exports aren't pulled into a runtime bundle and no longer get false-positive same-entry errors. - `SOURCE_EXTENSIONS` now includes `.mts` and `.cts` so entries and barrels using those extensions are followed correctly. - The ESLint plugin changeset wording matches the `error` severity of the recommended config. - Adds regression fixtures and RuleTester cases for `export type …` at both entries and for a file only reachable via a type-only edge. Signed-off-by: Marat Dyatko Made-with: Cursor --- .../add-no-self-package-imports-lint-rule.md | 2 +- packages/eslint-plugin/lib/visitImports.js | 2 +- .../rules/no-self-package-imports.js | 17 +++++++++++--- .../self-import-pkg/src/alpha/index.ts | 1 + .../self-import-pkg/src/alpha/typeRef.ts | 17 ++++++++++++++ .../src/no-self-package-imports.test.ts | 23 +++++++++++++++++++ 6 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/typeRef.ts diff --git a/.changeset/add-no-self-package-imports-lint-rule.md b/.changeset/add-no-self-package-imports-lint-rule.md index 743c5d73d0..89d9658038 100644 --- a/.changeset/add-no-self-package-imports-lint-rule.md +++ b/.changeset/add-no-self-package-imports-lint-rule.md @@ -2,4 +2,4 @@ '@backstage/eslint-plugin': minor --- -Added a new `no-self-package-imports` lint rule that warns when a package imports itself by its own name instead of using a relative path. This pattern causes circular initialization errors in bundled ESM and with `jest.requireActual`. +Added a new `no-self-package-imports` lint rule, enabled as `error` in the recommended config, that reports when a package imports itself by its own name instead of using a relative path. This pattern causes circular initialization errors in bundled ESM and with `jest.requireActual`. diff --git a/packages/eslint-plugin/lib/visitImports.js b/packages/eslint-plugin/lib/visitImports.js index 85c5ac0568..e4df5a8f32 100644 --- a/packages/eslint-plugin/lib/visitImports.js +++ b/packages/eslint-plugin/lib/visitImports.js @@ -111,7 +111,7 @@ function getImportInfo(node) { return { path: pathNode.value, node: pathNode, - kind: anyNode.importKind ?? 'value', + kind: anyNode.importKind ?? anyNode.exportKind ?? 'value', }; } diff --git a/packages/eslint-plugin/rules/no-self-package-imports.js b/packages/eslint-plugin/rules/no-self-package-imports.js index 3e42953125..0ea55e5d20 100644 --- a/packages/eslint-plugin/rules/no-self-package-imports.js +++ b/packages/eslint-plugin/rules/no-self-package-imports.js @@ -28,7 +28,16 @@ const getPackages = require('../lib/getPackages'); * @property {string} sourceFile - The source file for the entry, relative to the package dir. */ -const SOURCE_EXTENSIONS = ['.ts', '.tsx', '.js', '.jsx', '.mjs', '.cjs']; +const SOURCE_EXTENSIONS = [ + '.ts', + '.tsx', + '.mts', + '.cts', + '.js', + '.jsx', + '.mjs', + '.cjs', +]; // Cache the per-package analysis across files lint invocations. The key is the // absolute package dir; the value is a Map from absolute file path to the set @@ -84,9 +93,11 @@ function resolveSourcePath(fromFile, specifier) { } // Matches `from '...'` specifiers in `import`/`export ... from` statements. -// Uses a non-greedy body so multi-line imports match cleanly. +// Uses a non-greedy body so multi-line imports match cleanly. The negative +// lookahead skips `import type` / `export type` statements because type-only +// edges are erased at runtime and can't pull files into a runtime bundle. const FROM_SPEC_RE = - /(?:^|[\s;}])(?:import|export)\b[^"';]*?\bfrom\s*["']([^"']+)["']/gm; + /(?:^|[\s;}])(?:import|export)\b(?!\s+type\b)[^"';]*?\bfrom\s*["']([^"']+)["']/gm; // Matches side-effect imports: `import '...';`. const SIDE_EFFECT_IMPORT_RE = /(?:^|[\s;}])import\s*["']([^"']+)["']/gm; diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/index.ts b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/index.ts index 3a1f81dd58..a4b52bcd7e 100644 --- a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/index.ts +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/index.ts @@ -17,3 +17,4 @@ export * from './refs'; export * from '../shared'; export * from '../next'; +export type { TypeOnly } from './typeRef'; diff --git a/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/typeRef.ts b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/typeRef.ts new file mode 100644 index 0000000000..0fcdda5d70 --- /dev/null +++ b/packages/eslint-plugin/src/__fixtures__/monorepo/packages/self-import-pkg/src/alpha/typeRef.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2026 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export type TypeOnly = { value: number }; diff --git a/packages/eslint-plugin/src/no-self-package-imports.test.ts b/packages/eslint-plugin/src/no-self-package-imports.test.ts index 773752503a..e87e41f581 100644 --- a/packages/eslint-plugin/src/no-self-package-imports.test.ts +++ b/packages/eslint-plugin/src/no-self-package-imports.test.ts @@ -123,6 +123,29 @@ ruleTester.run(RULE, rule, { code: `import type { Foo } from '@internal/self-import-pkg/alpha'`, filename: path.join(PKG_DIR, 'src/alpha/refs.ts'), }, + // `export type { ... } from` is also a type-only statement: the TS AST + // marks it with `exportKind: 'type'`, and the emitted JS has no runtime + // edge. Both same-entry and cross-entry forms must be skipped. + { + code: `export type { Foo } from '@internal/self-import-pkg'`, + filename: path.join(PKG_DIR, 'src/index.ts'), + }, + { + code: `export type { Foo } from '@internal/self-import-pkg/alpha'`, + filename: path.join(PKG_DIR, 'src/alpha/refs.ts'), + }, + // `src/alpha/typeRef.ts` is only reachable from the `./alpha` barrel via + // an `export type { ... } from './typeRef'` edge. Since type-only edges + // are erased at runtime, the file isn't part of any entry's bundle and + // self-imports from it must be skipped as orphans. + { + code: `import { foo } from '@internal/self-import-pkg/alpha'`, + filename: path.join(PKG_DIR, 'src/alpha/typeRef.ts'), + }, + { + code: `import { foo } from '@internal/self-import-pkg'`, + filename: path.join(PKG_DIR, 'src/alpha/typeRef.ts'), + }, ], invalid: [ // Same-entry self-imports are always errors because they create circular