mirror of
https://github.com/actualbudget/actual.git
synced 2026-04-29 02:54:09 -05:00
Enhance no-cross-package-imports rule to support hard-banned imports and relative path resolution. Introduce checks for exports from undeclared dependencies and add tests for new behaviors, including hard bans on specific package imports.
This commit is contained in:
@@ -52,6 +52,16 @@ void runClassic(
|
||||
code: 'const core = require("@actual-app/core");',
|
||||
filename: 'packages/desktop-client/src/test.js',
|
||||
},
|
||||
// export { foo } from declared dep is allowed
|
||||
{
|
||||
code: 'export { something } from "@actual-app/core";',
|
||||
filename: 'packages/desktop-client/src/index.ts',
|
||||
},
|
||||
// export * from declared dep is allowed
|
||||
{
|
||||
code: 'export * from "@actual-app/core";',
|
||||
filename: 'packages/desktop-client/src/index.ts',
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
// @actual-app/components has no internal deps — cannot import @actual-app/core
|
||||
@@ -82,13 +92,13 @@ void runClassic(
|
||||
},
|
||||
],
|
||||
},
|
||||
// @actual-app/core cannot import @actual-app/web
|
||||
// @actual-app/core cannot import @actual-app/web (hard ban)
|
||||
{
|
||||
code: 'import { Component } from "@actual-app/web";',
|
||||
filename: 'packages/loot-core/src/server/main.ts',
|
||||
errors: [
|
||||
{
|
||||
messageId: 'noCrossPackageImport',
|
||||
messageId: 'hardBannedImport',
|
||||
data: {
|
||||
currentPackage: '@actual-app/core',
|
||||
importedPackage: '@actual-app/web',
|
||||
@@ -138,6 +148,48 @@ void runClassic(
|
||||
},
|
||||
],
|
||||
},
|
||||
// export * from undeclared dep is blocked
|
||||
{
|
||||
code: 'export * from "@actual-app/web";',
|
||||
filename: 'packages/component-library/src/index.ts',
|
||||
errors: [
|
||||
{
|
||||
messageId: 'noCrossPackageImport',
|
||||
data: {
|
||||
currentPackage: '@actual-app/components',
|
||||
importedPackage: '@actual-app/web',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
// export { foo } from undeclared dep is blocked
|
||||
{
|
||||
code: 'export { Page } from "@actual-app/web";',
|
||||
filename: 'packages/component-library/src/index.ts',
|
||||
errors: [
|
||||
{
|
||||
messageId: 'noCrossPackageImport',
|
||||
data: {
|
||||
currentPackage: '@actual-app/components',
|
||||
importedPackage: '@actual-app/web',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
// export * from @actual-app/web in loot-core triggers hard ban
|
||||
{
|
||||
code: 'export * from "@actual-app/web";',
|
||||
filename: 'packages/loot-core/src/index.ts',
|
||||
errors: [
|
||||
{
|
||||
messageId: 'hardBannedImport',
|
||||
data: {
|
||||
currentPackage: '@actual-app/core',
|
||||
importedPackage: '@actual-app/web',
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
},
|
||||
{
|
||||
|
||||
@@ -26,29 +26,51 @@ function findMonorepoRoot() {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Computes a monorepo-root-relative path for reliable package detection.
|
||||
* This avoids false matches when the checkout itself is under a directory named "packages/".
|
||||
*/
|
||||
function toMonorepoRelative(filename) {
|
||||
const monoRoot = findMonorepoRoot();
|
||||
if (!monoRoot) return null;
|
||||
|
||||
const normalized = filename.replace(/\\/g, '/');
|
||||
const normalizedRoot = monoRoot.replace(/\\/g, '/');
|
||||
|
||||
if (normalized.startsWith(normalizedRoot + '/')) {
|
||||
return normalized.substring(normalizedRoot.length + 1);
|
||||
}
|
||||
|
||||
// For relative paths, try to resolve against monorepo root
|
||||
const resolved = path.resolve(monoRoot, filename).replace(/\\/g, '/');
|
||||
if (resolved.startsWith(normalizedRoot + '/')) {
|
||||
return resolved.substring(normalizedRoot.length + 1);
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Finds the package info for a given filename by locating the nearest
|
||||
* packages/<dir>/package.json in the file path.
|
||||
* packages/<dir>/package.json in the file path, anchored to the monorepo root.
|
||||
*/
|
||||
function getPackageInfo(filename) {
|
||||
const normalized = filename.replace(/\\/g, '/');
|
||||
const match = normalized.match(/packages\/([^/]+)\//);
|
||||
const relative = toMonorepoRelative(filename);
|
||||
if (!relative) return null;
|
||||
|
||||
const match = relative.match(/^packages\/([^/]+)\//);
|
||||
if (!match) return null;
|
||||
|
||||
const packageDir = match[1];
|
||||
if (packageCache.has(packageDir)) return packageCache.get(packageDir);
|
||||
|
||||
// Try to find package.json using the path as-is first (works for absolute paths)
|
||||
const packagesIndex = normalized.indexOf('packages/' + packageDir + '/');
|
||||
const root = normalized.substring(0, packagesIndex);
|
||||
let pkgJsonPath = path.join(root, 'packages', packageDir, 'package.json');
|
||||
|
||||
// If not found (e.g. relative path with different cwd), use monorepo root
|
||||
if (!fs.existsSync(pkgJsonPath)) {
|
||||
const monoRoot = findMonorepoRoot();
|
||||
if (!monoRoot) return null;
|
||||
pkgJsonPath = path.join(monoRoot, 'packages', packageDir, 'package.json');
|
||||
}
|
||||
const monoRoot = findMonorepoRoot();
|
||||
const pkgJsonPath = path.join(
|
||||
monoRoot,
|
||||
'packages',
|
||||
packageDir,
|
||||
'package.json',
|
||||
);
|
||||
|
||||
try {
|
||||
const pkgJson = JSON.parse(fs.readFileSync(pkgJsonPath, 'utf8'));
|
||||
@@ -96,9 +118,12 @@ function extractActualPackageName(importSource) {
|
||||
function resolveRelativeCrossPackage(importSource, filename, currentDirName) {
|
||||
if (!importSource.startsWith('.')) return null;
|
||||
|
||||
const fileDir = path.dirname(filename).replace(/\\/g, '/');
|
||||
const relative = toMonorepoRelative(filename);
|
||||
if (!relative) return null;
|
||||
|
||||
const fileDir = path.posix.dirname(relative);
|
||||
const resolved = path.posix.normalize(path.posix.join(fileDir, importSource));
|
||||
const match = resolved.match(/packages\/([^/]+)\//);
|
||||
const match = resolved.match(/^packages\/([^/]+)\//);
|
||||
if (!match) return null;
|
||||
|
||||
const targetDir = match[1];
|
||||
@@ -131,6 +156,8 @@ module.exports = {
|
||||
messages: {
|
||||
noCrossPackageImport:
|
||||
'Package "{{currentPackage}}" does not declare a dependency on "{{importedPackage}}". Add it to dependencies in package.json or remove the import.',
|
||||
hardBannedImport:
|
||||
'Package "{{currentPackage}}" must never import "{{importedPackage}}". This boundary is enforced unconditionally to keep {{currentPackage}} platform-agnostic.',
|
||||
},
|
||||
},
|
||||
|
||||
@@ -141,6 +168,15 @@ module.exports = {
|
||||
// Not inside a recognized package — nothing to check
|
||||
if (!pkgInfo) return {};
|
||||
|
||||
// Hard-banned import pairs: these are always forbidden regardless of package.json deps
|
||||
const HARD_BANS = [{ from: '@actual-app/core', to: '@actual-app/web' }];
|
||||
|
||||
function isHardBanned(currentPkg, targetPkg) {
|
||||
return HARD_BANS.some(
|
||||
ban => ban.from === currentPkg && ban.to === targetPkg,
|
||||
);
|
||||
}
|
||||
|
||||
function checkImportSource(node, source) {
|
||||
if (typeof source !== 'string') return;
|
||||
|
||||
@@ -149,6 +185,18 @@ module.exports = {
|
||||
if (importedPackage) {
|
||||
if (importedPackage === pkgInfo.name) return;
|
||||
|
||||
if (isHardBanned(pkgInfo.name, importedPackage)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'hardBannedImport',
|
||||
data: {
|
||||
currentPackage: pkgInfo.name,
|
||||
importedPackage,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (!pkgInfo.allowedDeps.has(importedPackage)) {
|
||||
context.report({
|
||||
node,
|
||||
@@ -170,6 +218,19 @@ module.exports = {
|
||||
);
|
||||
if (targetDir) {
|
||||
const targetPkgName = getPackageNameForDir(targetDir) || targetDir;
|
||||
|
||||
if (isHardBanned(pkgInfo.name, targetPkgName)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'hardBannedImport',
|
||||
data: {
|
||||
currentPackage: pkgInfo.name,
|
||||
importedPackage: targetPkgName,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (!pkgInfo.allowedDeps.has(targetPkgName)) {
|
||||
context.report({
|
||||
node,
|
||||
@@ -206,6 +267,20 @@ module.exports = {
|
||||
checkImportSource(node, node.source.value);
|
||||
}
|
||||
},
|
||||
|
||||
// export { foo } from '...'
|
||||
ExportNamedDeclaration(node) {
|
||||
if (node.source) {
|
||||
checkImportSource(node, node.source.value);
|
||||
}
|
||||
},
|
||||
|
||||
// export * from '...'
|
||||
ExportAllDeclaration(node) {
|
||||
if (node.source) {
|
||||
checkImportSource(node, node.source.value);
|
||||
}
|
||||
},
|
||||
};
|
||||
},
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user