diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 54af935a..eaa7bf0d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -210,7 +210,7 @@ npm run lint - Please adhere to the coding style, patterns, and conventions used throughout the existing codebase. - Consult [GEMINI.md](https://github.com/google-gemini/gemini-cli/blob/main/GEMINI.md) (typically found in the project root) for specific instructions related to AI-assisted development, including conventions for React, comments, and Git usage. -- **Imports:** Pay special attention to import paths. The project uses `eslint-rules/no-relative-cross-package-imports.js` to enforce restrictions on relative imports between packages. +- **Imports:** Pay special attention to import paths. The project uses ESLint to enforce restrictions on relative imports between packages. ### Project Structure diff --git a/eslint-rules/no-relative-cross-package-imports.js b/eslint-rules/no-relative-cross-package-imports.js deleted file mode 100644 index ab3ed91c..00000000 --- a/eslint-rules/no-relative-cross-package-imports.js +++ /dev/null @@ -1,159 +0,0 @@ -/** - * @license - * Copyright 2025 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * @fileoverview Disallows relative imports between specified monorepo packages. - */ -'use strict'; - -import path from 'node:path'; -import fs from 'node:fs'; - -/** - * Finds the package name by searching for the nearest `package.json` file - * in the directory hierarchy, starting from the given file's directory - * and moving upwards until the specified root directory is reached. - * It reads the `package.json` and extracts the `name` property. - * - * @requires module:path Node.js path module - * @requires module:fs Node.js fs module - * - * @param {string} filePath - The path (absolute or relative) to a file within the potential package structure. - * The search starts from the directory containing this file. - * @param {string} root - The absolute path to the root directory of the project/monorepo. - * The upward search stops when this directory is reached. - * @returns {string | undefined | null} The value of the `name` field from the first `package.json` found. - * Returns `undefined` if the `name` field doesn't exist in the found `package.json`. - * Returns `null` if no `package.json` is found before reaching the `root` directory. - * @throws {Error} Can throw an error if `fs.readFileSync` fails (e.g., permissions) or if `JSON.parse` fails on invalid JSON content. - */ -function findPackageName(filePath, root) { - let currentDir = path.dirname(path.resolve(filePath)); - while (currentDir !== root) { - const parentDir = path.dirname(currentDir); - const packageJsonPath = path.join(currentDir, 'package.json'); - if (fs.existsSync(packageJsonPath)) { - const pkg = JSON.parse(fs.readFileSync(packageJsonPath, 'utf-8')); - return pkg.name; - } - - // Move up one level - currentDir = parentDir; - // Safety break if we somehow reached the root directly in the loop condition (less likely with path.resolve) - if (path.dirname(currentDir) === currentDir) break; - } - - return null; // Not found within the expected structure -} - -export default { - meta: { - type: 'problem', - docs: { - description: 'Disallow relative imports between packages.', - category: 'Best Practices', - recommended: 'error', - }, - fixable: 'code', - schema: [ - { - type: 'object', - properties: { - root: { - type: 'string', - description: - 'Absolute path to the root of all relevant packages to consider.', - }, - }, - required: ['root'], - additionalProperties: false, - }, - ], - messages: { - noRelativePathsForCrossPackageImport: - "Relative import '{{importedPath}}' crosses package boundary from '{{importingPackage}}' to '{{importedPackage}}'. Use a direct package import ('{{importedPackage}}') instead.", - relativeImportIsInvalidPackage: - "Relative import '{{importedPath}}' does not reference a valid package. All source must be in a package directory.", - }, - }, - - create(context) { - const options = context.options[0] || {}; - const allPackagesRoot = options.root; - - const currentFilePath = context.filename; - if ( - !currentFilePath || - currentFilePath === '' || - currentFilePath === '' - ) { - // Skip if filename is not available (e.g., linting raw text) - return {}; - } - - const currentPackage = findPackageName(currentFilePath, allPackagesRoot); - - // If the current file isn't inside a package structure, don't apply the rule - if (!currentPackage) { - return {}; - } - - return { - ImportDeclaration(node) { - const importingPackage = currentPackage; - const importedPath = node.source.value; - - // Only interested in relative paths - if ( - !importedPath || - typeof importedPath !== 'string' || - !importedPath.startsWith('.') - ) { - return; - } - - // Resolve the absolute path of the imported module - const absoluteImportPath = path.resolve( - path.dirname(currentFilePath), - importedPath, - ); - - // Find the package information for the imported file - const importedPackage = findPackageName( - absoluteImportPath, - allPackagesRoot, - ); - - // If the imported file isn't in a recognized package, report issue - if (!importedPackage) { - context.report({ - node: node.source, - messageId: 'relativeImportIsInvalidPackage', - data: { importedPath: importedPath }, - }); - return; - } - - // The core check: Are the source and target packages different? - if (currentPackage !== importedPackage) { - // We found a relative import crossing package boundaries - context.report({ - node: node.source, // Report the error on the source string literal - messageId: 'noRelativePathsForCrossPackageImport', - data: { - importedPath, - importedPackage, - importingPackage, - }, - fix(fixer) { - return fixer.replaceText(node.source, `'${importedPackage}'`); - }, - }); - } - }, - }; - }, -}; diff --git a/eslint.config.js b/eslint.config.js index 29aa23dc..169bbd17 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -12,7 +12,6 @@ import prettierConfig from 'eslint-config-prettier'; import importPlugin from 'eslint-plugin-import'; import globals from 'globals'; import licenseHeader from 'eslint-plugin-license-header'; -import noRelativeCrossPackageImports from './eslint-rules/no-relative-cross-package-imports.js'; import path from 'node:path'; // Use node: prefix for built-ins import url from 'node:url'; @@ -34,7 +33,6 @@ export default tseslint.config( 'packages/core/dist/**', 'packages/server/dist/**', 'packages/vscode-ide-companion/dist/**', - 'eslint-rules/*', 'bundle/**', ], }, @@ -72,6 +70,14 @@ export default tseslint.config( { // General overrides and rules for the project (TS/TSX files) files: ['packages/*/src/**/*.{ts,tsx}'], // Target only TS/TSX in the cli package + plugins: { + import: importPlugin, + }, + settings: { + 'import/resolver': { + node: true, + }, + }, languageOptions: { globals: { ...globals.node, @@ -106,6 +112,13 @@ export default tseslint.config( caughtErrorsIgnorePattern: '^_', }, ], + 'import/no-internal-modules': [ + 'error', + { + allow: ['react-dom/test-utils', 'memfs/lib/volume.js', 'yargs/**'], + }, + ], + 'import/no-relative-packages': 'error', 'no-cond-assign': 'error', 'no-debugger': 'error', 'no-duplicate-case': 'error', @@ -213,24 +226,4 @@ export default tseslint.config( ], }, }, - // Custom eslint rules for this repo - { - files: ['packages/**/*.{js,jsx,ts,tsx}'], - plugins: { - custom: { - rules: { - 'no-relative-cross-package-imports': noRelativeCrossPackageImports, - }, - }, - }, - rules: { - // Enable and configure your custom rule - 'custom/no-relative-cross-package-imports': [ - 'error', - { - root: path.join(projectRoot, 'packages'), - }, - ], - }, - }, );