mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-16 21:19:43 +02:00
chore: Add review/lint rule to discourage open-ended fields in nodes (warn level) (#39959)
Co-authored-by: n8n-cat-bot[bot] <n8n-cat-bot[bot]@users.noreply.github.com>
This commit is contained in:
n8n-cat-bot[bot]
GitHub
parent
8ee5cf1845
commit
24621ad4ca
|
|
@ -77,6 +77,7 @@ export default [
|
|||
| [node-connection-type-literal](docs/rules/node-connection-type-literal.md) | Disallow string literals in node description `inputs`/`outputs` — use `NodeConnectionTypes` enum instead | ✅ ☑️ | | 🔧 | | |
|
||||
| [node-registration-complete](docs/rules/node-registration-complete.md) | Ensure every `.node.ts` file in the `nodes/` directory is registered in the "n8n.nodes" array of package.json | | ✅ ☑️ | | | |
|
||||
| [node-usable-as-tool](docs/rules/node-usable-as-tool.md) | Ensure node classes have usableAsTool property | ✅ ☑️ | | 🔧 | | |
|
||||
| [open-ended-node-fields](docs/rules/open-ended-node-fields.md) | Discourage optional open-ended JSON fields in node properties | | ✅ ☑️ | | | |
|
||||
| [package-name-convention](docs/rules/package-name-convention.md) | Enforce correct package naming convention for n8n community nodes | ✅ ☑️ | | | 💡 | |
|
||||
| [require-files-array](docs/rules/require-files-array.md) | Require a non-empty "files" array in package.json | | ✅ ☑️ | | | |
|
||||
| [require-homepage](docs/rules/require-homepage.md) | Require a "homepage" field with a valid URL in package.json | | ✅ ☑️ | | | |
|
||||
|
|
|
|||
|
|
@ -0,0 +1,35 @@
|
|||
# Discourage optional open-ended JSON fields in node properties (`@n8n/community-nodes/open-ended-node-fields`)
|
||||
|
||||
⚠️ This rule _warns_ in the following configs: ✅ `recommended`, ☑️ `recommendedWithoutN8nCloudSupport`.
|
||||
|
||||
<!-- end auto-generated rule header -->
|
||||
|
||||
## Rule Details
|
||||
|
||||
This rule warns about optional top-level JSON properties in node descriptions. A JSON configuration field can hide parameters from the editor. Typed parameters make configuration easier to find, validate, and change.
|
||||
|
||||
Place optional parameters under Additional Options or Additional Fields. The rule does not report required JSON properties or typed collection options.
|
||||
|
||||
## Examples
|
||||
|
||||
### Incorrect
|
||||
|
||||
```typescript
|
||||
properties: [
|
||||
{ displayName: 'Configuration', name: 'configuration', type: 'json', default: '{}' },
|
||||
];
|
||||
```
|
||||
|
||||
### Correct
|
||||
|
||||
```typescript
|
||||
properties: [
|
||||
{
|
||||
displayName: 'Additional Fields',
|
||||
name: 'additionalFields',
|
||||
type: 'collection',
|
||||
default: {},
|
||||
options: [{ displayName: 'Limit', name: 'limit', type: 'number', default: 10 }],
|
||||
},
|
||||
];
|
||||
```
|
||||
|
|
@ -29,6 +29,7 @@ const configs = {
|
|||
'@n8n/community-nodes/no-deprecated-workflow-functions': 'error',
|
||||
'@n8n/community-nodes/no-emoji-in-options': 'error',
|
||||
'@n8n/community-nodes/node-usable-as-tool': 'error',
|
||||
'@n8n/community-nodes/open-ended-node-fields': 'warn',
|
||||
'@n8n/community-nodes/package-name-convention': 'error',
|
||||
'@n8n/community-nodes/credential-test-required': 'error',
|
||||
'@n8n/community-nodes/no-credential-reuse': 'error',
|
||||
|
|
@ -79,6 +80,7 @@ const configs = {
|
|||
'@n8n/community-nodes/no-deprecated-workflow-functions': 'error',
|
||||
'@n8n/community-nodes/no-emoji-in-options': 'error',
|
||||
'@n8n/community-nodes/node-usable-as-tool': 'error',
|
||||
'@n8n/community-nodes/open-ended-node-fields': 'warn',
|
||||
'@n8n/community-nodes/package-name-convention': 'error',
|
||||
'@n8n/community-nodes/credential-test-required': 'error',
|
||||
'@n8n/community-nodes/no-credential-reuse': 'error',
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import { NodeClassDescriptionIconMissingRule } from './node-class-description-ic
|
|||
import { NodeConnectionTypeLiteralRule } from './node-connection-type-literal.js';
|
||||
import { NodeRegistrationCompleteRule } from './node-registration-complete.js';
|
||||
import { NodeUsableAsToolRule } from './node-usable-as-tool.js';
|
||||
import { OpenEndedNodeFieldsRule } from './open-ended-node-fields.js';
|
||||
import { PackageNameConventionRule } from './package-name-convention.js';
|
||||
import { RequireFilesArrayRule } from './require-files-array.js';
|
||||
import { RequireHomepageRule } from './require-homepage.js';
|
||||
|
|
@ -54,6 +55,7 @@ export const rules = {
|
|||
'no-deprecated-workflow-functions': NoDeprecatedWorkflowFunctionsRule,
|
||||
'no-emoji-in-options': NoEmojiInOptionsRule,
|
||||
'node-usable-as-tool': NodeUsableAsToolRule,
|
||||
'open-ended-node-fields': OpenEndedNodeFieldsRule,
|
||||
'package-name-convention': PackageNameConventionRule,
|
||||
'credential-test-required': CredentialTestRequiredRule,
|
||||
'credential-unnecessary-password': CredentialUnnecessaryPasswordRule,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,122 @@
|
|||
import { Linter } from 'eslint';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
import { configs } from '../plugin.js';
|
||||
|
||||
const linter = new Linter();
|
||||
|
||||
function createNodeCode(property: string): string {
|
||||
return `
|
||||
export class TestNode extends Node {
|
||||
description = {
|
||||
displayName: 'Test Node',
|
||||
name: 'testNode',
|
||||
group: ['transform'],
|
||||
version: 1,
|
||||
inputs: ['main'],
|
||||
outputs: ['main'],
|
||||
properties: [${property}],
|
||||
};
|
||||
}`;
|
||||
}
|
||||
|
||||
function lintNode(code: string, config: keyof typeof configs) {
|
||||
return linter.verify(code, [{ ...configs[config], files: ['**/*.node.ts'] }], {
|
||||
filename: 'src/rules/TestNode.node.ts',
|
||||
});
|
||||
}
|
||||
|
||||
describe.each(['recommended', 'recommendedWithoutN8nCloudSupport'] as const)(
|
||||
'%s node property review',
|
||||
(config) => {
|
||||
// CE-1879: Warn when an optional JSON field replaces typed node parameters.
|
||||
it('warns about an optional JSON configuration catch-all', () => {
|
||||
const code = createNodeCode(`{
|
||||
displayName: 'Configuration',
|
||||
name: 'configuration',
|
||||
type: 'json',
|
||||
required: false,
|
||||
default: '{}',
|
||||
}`);
|
||||
|
||||
const warnings = lintNode(code, config).filter(
|
||||
({ severity, message }) => severity === 1 && /Additional (Options|Fields)/i.test(message),
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([
|
||||
expect.objectContaining({
|
||||
severity: 1,
|
||||
message: expect.stringMatching(/Additional (Options|Fields)/i),
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it('does not warn about typed parameters inside Additional Fields', () => {
|
||||
const code = createNodeCode(`{
|
||||
displayName: 'Additional Fields',
|
||||
name: 'additionalFields',
|
||||
type: 'collection',
|
||||
default: {},
|
||||
options: [{ displayName: 'Limit', name: 'limit', type: 'number', default: 10 }],
|
||||
}`);
|
||||
|
||||
const messages = lintNode(code, config);
|
||||
expect(messages).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
ruleId: '@n8n/community-nodes/require-node-description-fields',
|
||||
}),
|
||||
]),
|
||||
);
|
||||
|
||||
const warnings = messages.filter(
|
||||
({ severity, message }) => severity === 1 && /Additional (Options|Fields)/i.test(message),
|
||||
);
|
||||
|
||||
expect(warnings).toEqual([]);
|
||||
});
|
||||
|
||||
it('warns when an open-ended JSON field does not declare required', () => {
|
||||
const code = createNodeCode(`{
|
||||
displayName: 'Configuration',
|
||||
name: 'configuration',
|
||||
type: 'json',
|
||||
default: '{}',
|
||||
}`);
|
||||
|
||||
expect(
|
||||
lintNode(code, config).filter(
|
||||
({ ruleId }) => ruleId === '@n8n/community-nodes/open-ended-node-fields',
|
||||
),
|
||||
).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('does not warn about required JSON fields', () => {
|
||||
const code = createNodeCode(`{
|
||||
displayName: 'Request Body',
|
||||
name: 'body',
|
||||
type: 'json',
|
||||
required: true,
|
||||
default: '{}',
|
||||
}`);
|
||||
|
||||
expect(
|
||||
lintNode(code, config).filter(
|
||||
({ ruleId }) => ruleId === '@n8n/community-nodes/open-ended-node-fields',
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not inspect classes that are not nodes', () => {
|
||||
const code = `class Helper {
|
||||
description = { properties: [{ type: 'json', required: false }] };
|
||||
}`;
|
||||
|
||||
expect(
|
||||
lintNode(code, config).filter(
|
||||
({ ruleId }) => ruleId === '@n8n/community-nodes/open-ended-node-fields',
|
||||
),
|
||||
).toEqual([]);
|
||||
});
|
||||
},
|
||||
);
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
import { AST_NODE_TYPES } from '@typescript-eslint/utils';
|
||||
|
||||
import {
|
||||
createRule,
|
||||
findArrayLiteralProperty,
|
||||
findNodeDescriptionObject,
|
||||
findObjectProperty,
|
||||
getBooleanLiteralValue,
|
||||
getStringLiteralValue,
|
||||
isFileType,
|
||||
isNodeTypeClass,
|
||||
} from '../utils/index.js';
|
||||
|
||||
export const OpenEndedNodeFieldsRule = createRule({
|
||||
name: 'open-ended-node-fields',
|
||||
meta: {
|
||||
type: 'suggestion',
|
||||
docs: {
|
||||
description: 'Discourage optional open-ended JSON fields in node properties',
|
||||
},
|
||||
messages: {
|
||||
optionalJson:
|
||||
'Optional JSON fields can hide configuration. Use typed parameters under Additional Options or Additional Fields instead.',
|
||||
},
|
||||
schema: [],
|
||||
},
|
||||
defaultOptions: [],
|
||||
create(context) {
|
||||
if (!isFileType(context.filename, '.node.ts')) return {};
|
||||
|
||||
return {
|
||||
ClassDeclaration(node) {
|
||||
if (!isNodeTypeClass(node)) return;
|
||||
|
||||
const description = findNodeDescriptionObject(node);
|
||||
if (!description) return;
|
||||
|
||||
const properties = findArrayLiteralProperty(description, 'properties');
|
||||
if (!properties) return;
|
||||
|
||||
for (const property of properties.elements) {
|
||||
if (property?.type !== AST_NODE_TYPES.ObjectExpression) continue;
|
||||
|
||||
const type = findObjectProperty(property, 'type');
|
||||
if (getStringLiteralValue(type?.value ?? null) !== 'json') continue;
|
||||
|
||||
const required = findObjectProperty(property, 'required');
|
||||
if (getBooleanLiteralValue(required?.value ?? null) === true) continue;
|
||||
|
||||
context.report({ node: property, messageId: 'optionalJson' });
|
||||
}
|
||||
},
|
||||
};
|
||||
},
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user