mirror of
https://github.com/actualbudget/actual.git
synced 2026-04-28 01:58:40 -05:00
[AI] Detect relative imports that cross package boundaries
Extend no-cross-package-imports rule to also catch relative imports like `../../desktop-client/src/...` that traverse into a different package directory. Resolves the import path against the file's directory and checks if the target package is declared as a dependency. Add oxlint-disable for the known intentional violation in component-library/.storybook/preview.tsx. https://claude.ai/code/session_01XjmtRs1P9Rg7FNJAYVcaZJ
This commit is contained in:
@@ -4,10 +4,12 @@ import type { Preview } from '@storybook/react-vite';
|
||||
|
||||
// Not ideal to import from desktop-client, but we need a source of truth for theme variables
|
||||
// TODO: this needs refactoring
|
||||
// oxlint-disable actual/no-cross-package-imports -- intentional cross-package import, needs refactoring
|
||||
import * as darkTheme from '../../desktop-client/src/style/themes/dark';
|
||||
import * as developmentTheme from '../../desktop-client/src/style/themes/development';
|
||||
import * as lightTheme from '../../desktop-client/src/style/themes/light';
|
||||
import * as midnightTheme from '../../desktop-client/src/style/themes/midnight';
|
||||
// oxlint-enable actual/no-cross-package-imports
|
||||
|
||||
const THEMES = {
|
||||
light: lightTheme,
|
||||
|
||||
@@ -22,11 +22,16 @@ void runClassic(
|
||||
code: 'import React from "react";',
|
||||
filename: 'packages/component-library/src/Button.tsx',
|
||||
},
|
||||
// Relative imports are always allowed
|
||||
// Relative imports within same package are allowed
|
||||
{
|
||||
code: 'import { helper } from "./utils";',
|
||||
filename: 'packages/component-library/src/Button.tsx',
|
||||
},
|
||||
// Relative import to parent within same package is allowed
|
||||
{
|
||||
code: 'import { helper } from "../../shared/utils";',
|
||||
filename: 'packages/loot-core/src/server/deep/file.ts',
|
||||
},
|
||||
// Files outside packages/ are not checked
|
||||
{
|
||||
code: 'import { something } from "@actual-app/core";',
|
||||
@@ -119,6 +124,20 @@ void runClassic(
|
||||
},
|
||||
],
|
||||
},
|
||||
// Relative import crossing into another package is blocked
|
||||
{
|
||||
code: 'import * as theme from "../../desktop-client/src/style/themes/dark";',
|
||||
filename: 'packages/component-library/.storybook/preview.tsx',
|
||||
errors: [
|
||||
{
|
||||
messageId: 'noCrossPackageImport',
|
||||
data: {
|
||||
currentPackage: '@actual-app/components',
|
||||
importedPackage: '@actual-app/web',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
|
||||
@@ -1,9 +1,12 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
// Module-level cache: packageDir -> { name: string, allowedDeps: Set<string> }
|
||||
// Module-level cache: packageDir -> { name: string, allowedDeps: Set<string>, dirName: string }
|
||||
const packageCache = new Map();
|
||||
|
||||
// Reverse map: directory name -> package name (e.g. 'desktop-client' -> '@actual-app/web')
|
||||
const dirToPackageName = new Map();
|
||||
|
||||
// Find monorepo root by walking up from this file's directory
|
||||
let monorepoRoot = null;
|
||||
function findMonorepoRoot() {
|
||||
@@ -64,8 +67,13 @@ function getPackageInfo(filename) {
|
||||
}
|
||||
}
|
||||
}
|
||||
const info = { name: pkgJson.name, allowedDeps: allowed };
|
||||
const info = {
|
||||
name: pkgJson.name,
|
||||
allowedDeps: allowed,
|
||||
dirName: packageDir,
|
||||
};
|
||||
packageCache.set(packageDir, info);
|
||||
dirToPackageName.set(packageDir, pkgJson.name);
|
||||
return info;
|
||||
} catch {
|
||||
return null;
|
||||
@@ -81,13 +89,42 @@ function extractActualPackageName(importSource) {
|
||||
return match ? match[1] : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* For a relative import, resolves which packages/<dir> it lands in.
|
||||
* Returns the target directory name if it crosses into a different package, null otherwise.
|
||||
*/
|
||||
function resolveRelativeCrossPackage(importSource, filename, currentDirName) {
|
||||
if (!importSource.startsWith('.')) return null;
|
||||
|
||||
const fileDir = path.dirname(filename).replace(/\\/g, '/');
|
||||
const resolved = path.posix.normalize(path.posix.join(fileDir, importSource));
|
||||
const match = resolved.match(/packages\/([^/]+)\//);
|
||||
if (!match) return null;
|
||||
|
||||
const targetDir = match[1];
|
||||
if (targetDir === currentDirName) return null;
|
||||
|
||||
return targetDir;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the package name for a directory, loading its package.json if needed.
|
||||
*/
|
||||
function getPackageNameForDir(targetDir) {
|
||||
if (dirToPackageName.has(targetDir)) return dirToPackageName.get(targetDir);
|
||||
|
||||
// Force loading the package info which populates dirToPackageName
|
||||
const info = getPackageInfo(`packages/${targetDir}/dummy.ts`);
|
||||
return info ? info.name : null;
|
||||
}
|
||||
|
||||
/** @type {import('eslint').Rule.RuleModule} */
|
||||
module.exports = {
|
||||
meta: {
|
||||
type: 'problem',
|
||||
docs: {
|
||||
description:
|
||||
'Disallow importing @actual-app/* packages not declared as dependencies in package.json',
|
||||
'Disallow importing from other packages unless declared as a dependency in package.json',
|
||||
},
|
||||
fixable: null,
|
||||
schema: [],
|
||||
@@ -107,21 +144,42 @@ module.exports = {
|
||||
function checkImportSource(node, source) {
|
||||
if (typeof source !== 'string') return;
|
||||
|
||||
// Check @actual-app/* imports
|
||||
const importedPackage = extractActualPackageName(source);
|
||||
if (!importedPackage) return;
|
||||
if (importedPackage) {
|
||||
if (importedPackage === pkgInfo.name) return;
|
||||
|
||||
// Self-import is allowed
|
||||
if (importedPackage === pkgInfo.name) return;
|
||||
if (!pkgInfo.allowedDeps.has(importedPackage)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'noCrossPackageImport',
|
||||
data: {
|
||||
currentPackage: pkgInfo.name,
|
||||
importedPackage,
|
||||
},
|
||||
});
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (!pkgInfo.allowedDeps.has(importedPackage)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'noCrossPackageImport',
|
||||
data: {
|
||||
currentPackage: pkgInfo.name,
|
||||
importedPackage,
|
||||
},
|
||||
});
|
||||
// Check relative imports that cross package boundaries
|
||||
const targetDir = resolveRelativeCrossPackage(
|
||||
source,
|
||||
filename,
|
||||
pkgInfo.dirName,
|
||||
);
|
||||
if (targetDir) {
|
||||
const targetPkgName = getPackageNameForDir(targetDir) || targetDir;
|
||||
if (!pkgInfo.allowedDeps.has(targetPkgName)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'noCrossPackageImport',
|
||||
data: {
|
||||
currentPackage: pkgInfo.name,
|
||||
importedPackage: targetPkgName,
|
||||
},
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user