mirror of
https://github.com/n8n-io/n8n.git
synced 2026-07-28 03:24:59 +02:00
fix: Allow devDependency imports in linter (#34335)
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.18.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.22.3) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.18.0) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
This commit is contained in:
parent
02af123574
commit
26caaa876f
|
|
@ -12,6 +12,15 @@ Prevents importing external dependencies that are not allowed on n8n Cloud. Comm
|
|||
|
||||
Relative imports (starting with `./` or `../`) are always allowed.
|
||||
|
||||
**Dev dependencies are permitted.** Modules listed in the package's
|
||||
`devDependencies` (e.g. `vitest`, or type-only imports from a types
|
||||
package) are never installed at runtime on n8n Cloud — only the built
|
||||
`dist/` is shipped — so they are not runtime dependencies and are exempt
|
||||
from this rule. (Runtime `dependencies` are separately forced to be
|
||||
empty by `no-runtime-dependencies`.) This rule targets runtime
|
||||
dependencies only: anything in `dependencies`, or any import not on the
|
||||
allowlist and not relative, is restricted — including in test files.
|
||||
|
||||
## Examples
|
||||
|
||||
### ❌ Incorrect
|
||||
|
|
|
|||
|
|
@ -1,7 +1,33 @@
|
|||
import { RuleTester } from '@typescript-eslint/rule-tester';
|
||||
import { mkdtempSync, writeFileSync, rmSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterAll } from 'vitest';
|
||||
|
||||
import { NoRestrictedImportsRule } from './no-restricted-imports.js';
|
||||
|
||||
// Fixture package whose devDependencies include `vitest` and `@vitest/expect`,
|
||||
// and whose runtime `dependencies` include `axios`. Used to verify the rule
|
||||
// allows dev-dependency imports but still restricts runtime dependencies.
|
||||
// Created synchronously because RuleTester.run reads the `filename` option at
|
||||
// module-eval time, before any test hooks fire.
|
||||
const fixtureDir = mkdtempSync(join(tmpdir(), 'n8n-restricted-imports-'));
|
||||
writeFileSync(
|
||||
join(fixtureDir, 'package.json'),
|
||||
JSON.stringify({
|
||||
name: 'n8n-nodes-devdep-fixture',
|
||||
version: '1.0.0',
|
||||
devDependencies: { vitest: '^1.0.0', '@vitest/expect': '^1.0.0' },
|
||||
dependencies: { axios: '^1.0.0' },
|
||||
}),
|
||||
);
|
||||
const fixtureTestFile = join(fixtureDir, '__tests__', 'MyNode.test.ts');
|
||||
const fixtureRealFile = join(fixtureDir, 'src', 'MyNode.node.ts');
|
||||
|
||||
afterAll(() => {
|
||||
rmSync(fixtureDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
const ruleTester = new RuleTester();
|
||||
|
||||
ruleTester.run('no-restricted-imports', NoRestrictedImportsRule, {
|
||||
|
|
@ -78,6 +104,31 @@ ruleTester.run('no-restricted-imports', NoRestrictedImportsRule, {
|
|||
{
|
||||
code: 'const workflow = await import(`n8n-workflow`);',
|
||||
},
|
||||
{
|
||||
name: 'devDependency (vitest) import is allowed in test files',
|
||||
filename: fixtureTestFile,
|
||||
code: 'import { describe } from "vitest";',
|
||||
},
|
||||
{
|
||||
name: 'scoped devDependency import is allowed in test files',
|
||||
filename: fixtureTestFile,
|
||||
code: 'import { expect } from "@vitest/expect";',
|
||||
},
|
||||
{
|
||||
name: 'devDependency require is allowed in test files',
|
||||
filename: fixtureTestFile,
|
||||
code: 'const { it } = require("vitest");',
|
||||
},
|
||||
{
|
||||
name: 'devDependency dynamic import is allowed in test files',
|
||||
filename: fixtureTestFile,
|
||||
code: 'const vitest = await import("vitest");',
|
||||
},
|
||||
{
|
||||
name: 'type-only devDependency import is allowed in node source',
|
||||
filename: fixtureRealFile,
|
||||
code: 'import type { Task } from "vitest";',
|
||||
},
|
||||
],
|
||||
invalid: [
|
||||
{
|
||||
|
|
@ -157,6 +208,18 @@ const lodash = require("lodash");`,
|
|||
code: 'const express = await import("express");',
|
||||
errors: [{ messageId: 'restrictedDynamicImport', data: { modulePath: 'express' } }],
|
||||
},
|
||||
{
|
||||
name: 'runtime dependency (axios) is still restricted in a real node file',
|
||||
filename: fixtureRealFile,
|
||||
code: 'import axios from "axios";',
|
||||
errors: [{ messageId: 'restrictedImport', data: { modulePath: 'axios' } }],
|
||||
},
|
||||
{
|
||||
name: 'runtime dependency (axios) stays restricted even in test files',
|
||||
filename: fixtureTestFile,
|
||||
code: 'import axios from "axios";',
|
||||
errors: [{ messageId: 'restrictedImport', data: { modulePath: 'axios' } }],
|
||||
},
|
||||
{
|
||||
code: 'const path = require(`path`);',
|
||||
errors: [{ messageId: 'restrictedRequire', data: { modulePath: 'path' } }],
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@ import {
|
|||
isDirectRequireCall,
|
||||
isRequireMemberCall,
|
||||
createRule,
|
||||
findPackageJson,
|
||||
readPackageJsonDevDependencies,
|
||||
} from '../utils/index.js';
|
||||
|
||||
const allowedModules = [
|
||||
|
|
@ -18,13 +20,20 @@ const allowedModules = [
|
|||
'@n8n/ai-node-sdk',
|
||||
];
|
||||
|
||||
const isModuleAllowed = (modulePath: string): boolean => {
|
||||
const isModuleAllowed = (modulePath: string, devDependencies: Set<string>): boolean => {
|
||||
if (modulePath.startsWith('./') || modulePath.startsWith('../')) return true;
|
||||
|
||||
const moduleName = modulePath.startsWith('@')
|
||||
? modulePath.split('/').slice(0, 2).join('/')
|
||||
: modulePath.split('/')[0];
|
||||
if (!moduleName) return true;
|
||||
// Dev dependencies (e.g. `vitest`) are never installed at runtime on n8n
|
||||
// Cloud, so they are not subject to this rule — it targets runtime
|
||||
// dependencies only. `no-runtime-dependencies` already enforces that the
|
||||
// package's `dependencies` field is empty, so any external package an
|
||||
// author uses must be a devDependency (bundled at build) or a
|
||||
// peerDependency (provided by the instance).
|
||||
if (devDependencies.has(moduleName)) return true;
|
||||
return allowedModules.includes(moduleName);
|
||||
};
|
||||
|
||||
|
|
@ -47,10 +56,14 @@ export const NoRestrictedImportsRule = createRule({
|
|||
},
|
||||
defaultOptions: [],
|
||||
create(context) {
|
||||
const devDependencies = readPackageJsonDevDependencies(
|
||||
findPackageJson(context.physicalFilename),
|
||||
);
|
||||
|
||||
return {
|
||||
ImportDeclaration(node) {
|
||||
const modulePath = getModulePath(node.source);
|
||||
if (modulePath && !isModuleAllowed(modulePath)) {
|
||||
if (modulePath && !isModuleAllowed(modulePath, devDependencies)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'restrictedImport',
|
||||
|
|
@ -63,7 +76,7 @@ export const NoRestrictedImportsRule = createRule({
|
|||
|
||||
ImportExpression(node) {
|
||||
const modulePath = getModulePath(node.source);
|
||||
if (modulePath && !isModuleAllowed(modulePath)) {
|
||||
if (modulePath && !isModuleAllowed(modulePath, devDependencies)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'restrictedDynamicImport',
|
||||
|
|
@ -77,7 +90,7 @@ export const NoRestrictedImportsRule = createRule({
|
|||
CallExpression(node) {
|
||||
if (isDirectRequireCall(node) || isRequireMemberCall(node)) {
|
||||
const modulePath = getModulePath(node.arguments[0] ?? null);
|
||||
if (modulePath && !isModuleAllowed(modulePath)) {
|
||||
if (modulePath && !isModuleAllowed(modulePath, devDependencies)) {
|
||||
context.report({
|
||||
node,
|
||||
messageId: 'restrictedRequire',
|
||||
|
|
|
|||
|
|
@ -73,19 +73,40 @@ function isValidPackageJson(obj: unknown): obj is { n8n?: PackageJsonN8n } {
|
|||
return typeof obj === 'object' && obj !== null;
|
||||
}
|
||||
|
||||
function readPackageJsonN8n(packageJsonPath: string): PackageJsonN8n {
|
||||
function readPackageJsonRaw(packageJsonPath: string): Record<string, unknown> | null {
|
||||
try {
|
||||
const content = readFileSync(packageJsonPath, 'utf8');
|
||||
const parsed: unknown = JSON.parse(content);
|
||||
if (isValidPackageJson(parsed)) {
|
||||
return parsed.n8n ?? {};
|
||||
}
|
||||
return {};
|
||||
return isValidPackageJson(parsed) ? (parsed as Record<string, unknown>) : null;
|
||||
} catch {
|
||||
return {};
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function readPackageJsonN8n(packageJsonPath: string): PackageJsonN8n {
|
||||
const parsed = readPackageJsonRaw(packageJsonPath);
|
||||
if (parsed) {
|
||||
const n8n = parsed.n8n;
|
||||
return typeof n8n === 'object' && n8n !== null ? (n8n as PackageJsonN8n) : {};
|
||||
}
|
||||
return {};
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the set of package names listed under `devDependencies` in the given
|
||||
* package.json. Dev dependencies are never installed at runtime on n8n Cloud
|
||||
* (only the built `dist/` is shipped), so importing them is not a runtime
|
||||
* dependency concern and is permitted by `no-restricted-imports`.
|
||||
*/
|
||||
export function readPackageJsonDevDependencies(packageJsonPath: string | null): Set<string> {
|
||||
if (!packageJsonPath) return new Set();
|
||||
const parsed = readPackageJsonRaw(packageJsonPath);
|
||||
if (!parsed) return new Set();
|
||||
const devDeps = parsed.devDependencies;
|
||||
if (typeof devDeps !== 'object' || devDeps === null) return new Set();
|
||||
return new Set(Object.keys(devDeps as Record<string, unknown>));
|
||||
}
|
||||
|
||||
function resolveN8nFilePaths(packageJsonPath: string, filePaths: string[]): string[] {
|
||||
const packageDir = dirname(packageJsonPath);
|
||||
const resolvedFiles: string[] = [];
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user