Skip to content

Commit 2bc61ca

Browse files
authored
feat: switch to n8n-node lint (full ESLint community-nodes rules) (#147)
* fix: guard against a missing dependencies field in npm registry response data.dependencies was typed as always present, but if the registry response ever omitted it, accessing ["n8n-workflow"] would throw a raw TypeError instead of the descriptive error this code is meant to give. * feat: switch to n8n-node lint (full ESLint community-nodes rules) oxlint + eslint-plugin-n8n-nodes-base only checked a narrow subset of n8n's community-node rules and missed real issues: no dependencies field validation, no auth-pattern checks, weaker type-safety enforcement. Switch to n8n-node lint (matching n8n-nodes-actual), which runs the official @n8n/eslint-plugin-community-nodes rules alongside TypeScript ESLint and eslint-plugin-import-x. Uses configWithoutCloudSupport (n8n.strict: false) rather than the strict-mode default, since the cloud-only restricted-imports/globals rules fire broadly on tests/ and scripts/ tooling that never ships to n8n Cloud - same choice n8n-nodes-actual already made. Fixes surfaced by the switch: - package.json declared "dependencies" (eventsource, form-data), which community nodes must not do (they'd get bundled into the host n8n instance). Both are genuinely needed at runtime, so vendor them via an esbuild post-build step (scripts/bundle-vendor-deps.mjs) that inlines just those two packages into the two compiled files that use them, leaving n8n-workflow and local imports as normal requires. Verified the resulting dist/ has zero remaining requires for either package, and validated end-to-end via the full docker-compose pipeline (real n8n + PocketBase, including the expired-token isolated-process test) with the bundled build. - 3 no-http-request-with-manual-auth findings in LoadOptions.ts are suppressed, not reverted: this project deliberately moved off httpRequestWithAuthentication (commit 8003c58) because n8n doesn't reliably invoke a credential's authenticate hook on scheduled executions, and the credential type no longer even declares one. - RequestBodyFunctions.ts now throws NodeOperationError instead of a plain Error when rethrowing a JSON parse failure. - PocketbaseTrigger.node.ts gained a subtitle and properly typed its EventSource handlers instead of using `any`. - Reordered PocketbaseHttp's operation options alphabetically (cosmetic only - default is set explicitly, unaffected by order). - Replaced no-explicit-any across test files with real types (or a justified inline suppression for the one test that deliberately feeds malformed input to check a runtime guard). - no-console is scoped off for scripts/ (CLI tooling) and the one integration spec that intentionally logs live e2e diagnostics. * test: assert NodeOperationError type, not just message, for invalid JSON body The message-only assertion would still pass if the parse-failure path regressed to throwing a plain Error again. * fix: use NodeOperationError consistently across all three parseBodyJson throw sites Only the JSON.parse catch-block rethrow was upgraded; the two validation throws still used a plain Error despite node being available as a parameter now, giving inconsistent error surfacing for what are all "invalid body JSON" failures in the same helper.
1 parent eeb8e72 commit 2bc61ca

15 files changed

Lines changed: 223 additions & 517 deletions

.oxlintrc.json

Lines changed: 0 additions & 12 deletions
This file was deleted.

eslint.config.mjs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { configWithoutCloudSupport } from '@n8n/node-cli/eslint';
2+
3+
export default [
4+
...configWithoutCloudSupport,
5+
{
6+
// scripts/ holds standalone CLI tooling (run via tsx, not bundled into the
7+
// node), where console output is the point rather than something to avoid.
8+
files: ['scripts/**/*.ts'],
9+
rules: {
10+
'no-console': 'off',
11+
},
12+
},
13+
{
14+
// Diagnostic output for a real e2e run against a live PocketBase instance.
15+
files: ['tests/PocketbaseTriggerIntegration.spec.ts'],
16+
rules: {
17+
'no-console': 'off',
18+
},
19+
},
20+
];

nodes/Common/GenericFunctions.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export async function pagination(
102102
requestOptions: DeclarativeRestApiSettings.ResultOptions,
103103
): Promise<INodeExecutionData[]> {
104104
const allIsActive = this.getNodeParameter("parameters.allElements", false) as boolean;
105-
let executions: INodeExecutionData[] = [];
105+
const executions: INodeExecutionData[] = [];
106106
let page: number = this.getNodeParameter("parameters.page", 1) as number;
107107
let totalPages: number = allIsActive ? Infinity : page;
108108

nodes/Common/LoadOptions.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ async function loadPocketBaseFields(
3838
credentials.username as string,
3939
credentials.password as string,
4040
);
41+
// eslint-disable-next-line @n8n/community-nodes/no-http-request-with-manual-auth -- httpRequestWithAuthentication relies on the credential's `authenticate` property, which this project deliberately removed (see commit 8003c58): n8n doesn't reliably invoke it during scheduled executions, causing 403s once the stored JWT expired. fetchPocketbaseToken replaces it with a reliable, cache-aware token fetch.
4142
const { fields } = (await this.helpers.httpRequest({
4243
url: `${normalizedUrl}/api/collections/${resource}`,
4344
method: "GET",
@@ -85,6 +86,7 @@ export const LoadOptions = {
8586
let totalPages: number = 0;
8687

8788
do {
89+
// eslint-disable-next-line @n8n/community-nodes/no-http-request-with-manual-auth -- see comment in loadPocketBaseFields above
8890
const { items: pageItems, totalPages: pageTotalPages } = (await this.helpers.httpRequest({
8991
url: `${normalizedUrl}/api/collections`,
9092
method: "GET",
@@ -159,6 +161,7 @@ export const LoadOptions = {
159161
const maxPages = 5; // Load up to 5 pages for the dropdown
160162

161163
do {
164+
// eslint-disable-next-line @n8n/community-nodes/no-http-request-with-manual-auth -- see comment in loadPocketBaseFields above
162165
const { items: pageItems, totalPages: pageTotalPages } = (await this.helpers.httpRequest({
163166
url: `${normalizedUrl}/api/collections/${resource}/records`,
164167
method: "GET",
@@ -174,7 +177,7 @@ export const LoadOptions = {
174177
if (items.length === 0) {
175178
return [
176179
{
177-
name: "No records found",
180+
name: 'No Records Found',
178181
value: "",
179182
},
180183
];

nodes/Common/RequestBodyFunctions.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1-
// eslint-disable-next-line @n8n/community-nodes/no-restricted-imports
1+
22
import FormData from "form-data";
33
import {
44
AssignmentCollectionValue,
55
IDataObject,
66
IExecuteSingleFunctions,
77
IHttpRequestOptions,
8+
INode,
9+
NodeOperationError,
810
} from "n8n-workflow";
911

1012
export async function prepareRequestBody(
@@ -37,7 +39,7 @@ export async function prepareRequestBody(
3739

3840
if (bodyType.includes("bodyJson")) {
3941
const bodyJson = this.getNodeParameter("bodyJson", "{}") as string | Record<string, unknown>;
40-
const parsedBody = parseBodyJson(bodyJson);
42+
const parsedBody = parseBodyJson(bodyJson, this.getNode());
4143
const filteredParsedBody: IDataObject = {};
4244
Object.entries(parsedBody).forEach(([key, value]) => {
4345
if (
@@ -80,7 +82,7 @@ export async function prepareRequestBody(
8082

8183
if (bodyType.includes("bodyJson")) {
8284
const bodyJson = this.getNodeParameter("bodyJson", "{}") as string | Record<string, unknown>;
83-
const parsedBody = parseBodyJson(bodyJson);
85+
const parsedBody = parseBodyJson(bodyJson, this.getNode());
8486
Object.entries(parsedBody).forEach(([key, value]) => {
8587
if (
8688
key &&
@@ -144,12 +146,15 @@ async function handleBinaryData(this: IExecuteSingleFunctions, formData: FormDat
144146
});
145147
}
146148

147-
function parseBodyJson(bodyJson: string | Record<string, unknown>): Record<string, unknown> {
149+
function parseBodyJson(
150+
bodyJson: string | Record<string, unknown>,
151+
node: INode,
152+
): Record<string, unknown> {
148153
if (
149154
typeof bodyJson !== "string" &&
150155
(typeof bodyJson !== "object" || bodyJson === null || Array.isArray(bodyJson))
151156
) {
152-
throw new Error("JSON Body must be a JSON object or string");
157+
throw new NodeOperationError(node, "JSON Body must be a JSON object or string");
153158
}
154159

155160
let parsed: unknown;
@@ -158,14 +163,14 @@ function parseBodyJson(bodyJson: string | Record<string, unknown>): Record<strin
158163
parsed = JSON.parse(bodyJson);
159164
} catch (error) {
160165
const message = error instanceof Error ? error.message : String(error);
161-
throw new Error(`Invalid JSON in Body: ${message}`);
166+
throw new NodeOperationError(node, `Invalid JSON in Body: ${message}`);
162167
}
163168
} else {
164169
parsed = bodyJson;
165170
}
166171

167172
if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {
168-
throw new Error("JSON Body must be a JSON object");
173+
throw new NodeOperationError(node, "JSON Body must be a JSON object");
169174
}
170175

171176
return parsed as Record<string, unknown>;

nodes/PocketbaseHttp/PocketbaseHttp.node.ts

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ export class PocketbaseHttp implements INodeType {
5656
name: "operation",
5757
type: "options",
5858
options: [
59+
{
60+
name: "Create",
61+
value: "create",
62+
action: "Create an element in your collection",
63+
routing: {
64+
request: {
65+
method: "POST",
66+
url: `=/api/collections/{{$parameter["resource"]}}/records`,
67+
},
68+
send: {
69+
preSend: [authenticatePreSend, recordCreatePreSendAction],
70+
},
71+
},
72+
},
5973
{
6074
name: "List/Search",
6175
value: "search",
@@ -76,44 +90,30 @@ export class PocketbaseHttp implements INodeType {
7690
},
7791
},
7892
{
79-
name: "View",
80-
value: "view",
81-
action: "View an element in your collection",
93+
name: "Update",
94+
value: "update",
95+
action: "Update an element in your collection",
8296
routing: {
8397
request: {
84-
method: "GET",
98+
method: "PATCH",
8599
url: `=/api/collections/{{$parameter["resource"]}}/records/{{$parameter["elementId"]}}`,
86100
},
87101
send: {
88-
preSend: [authenticatePreSend, recordViewPreSendAction],
89-
},
90-
},
91-
},
92-
{
93-
name: "Create",
94-
value: "create",
95-
action: "Create an element in your collection",
96-
routing: {
97-
request: {
98-
method: "POST",
99-
url: `=/api/collections/{{$parameter["resource"]}}/records`,
100-
},
101-
send: {
102-
preSend: [authenticatePreSend, recordCreatePreSendAction],
102+
preSend: [authenticatePreSend, recordUpdatePreSendAction],
103103
},
104104
},
105105
},
106106
{
107-
name: "Update",
108-
value: "update",
109-
action: "Update an element in your collection",
107+
name: "View",
108+
value: "view",
109+
action: "View an element in your collection",
110110
routing: {
111111
request: {
112-
method: "PATCH",
112+
method: "GET",
113113
url: `=/api/collections/{{$parameter["resource"]}}/records/{{$parameter["elementId"]}}`,
114114
},
115115
send: {
116-
preSend: [authenticatePreSend, recordUpdatePreSendAction],
116+
preSend: [authenticatePreSend, recordViewPreSendAction],
117117
},
118118
},
119119
},

nodes/PocketbaseTrigger/PocketbaseTrigger.node.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ import {
33
ITriggerResponse,
44
INodeType,
55
INodeTypeDescription,
6+
IDataObject,
67
NodeConnectionTypes,
78
} from "n8n-workflow";
8-
import { EventSource } from "eventsource";
9+
import { EventSource, ErrorEvent as EventSourceErrorEvent } from "eventsource";
910
import { LoadOptions } from "../Common/LoadOptions";
1011

1112
export class PocketbaseTrigger implements INodeType {
@@ -16,6 +17,7 @@ export class PocketbaseTrigger implements INodeType {
1617
group: ["trigger"],
1718
version: 1,
1819
description: "Handle Pocketbase events via SSE (Beta)",
20+
subtitle: '={{$parameter["collection"]}}',
1921
defaults: {
2022
name: "Pocketbase Trigger",
2123
},
@@ -29,15 +31,15 @@ export class PocketbaseTrigger implements INodeType {
2931
],
3032
properties: [
3133
{
32-
displayName: "Collection Name",
34+
displayName: 'Collection Name or ID',
3335
name: "collection",
3436
type: "options",
3537
typeOptions: {
3638
loadOptionsMethod: "getCollections",
3739
},
3840
default: "",
3941
required: true,
40-
description: "The name of the collection to watch for changes",
42+
description: 'The name of the collection to watch for changes. Choose from the list, or specify an ID using an <a href="https://docs.n8n.io/code/expressions/">expression</a>.',
4143
},
4244
{
4345
displayName: "Events",
@@ -62,6 +64,7 @@ export class PocketbaseTrigger implements INodeType {
6264
description: "The events to trigger the node",
6365
},
6466
],
67+
usableAsTool: true,
6568
};
6669

6770
methods = {
@@ -108,7 +111,7 @@ function subscribeToPocketbaseSSE(
108111
const MAX_RECONNECT_ATTEMPTS = 50;
109112
let consecutiveFailures = 0;
110113

111-
const onConnect = async (e: any) => {
114+
const onConnect = async (e: MessageEvent) => {
112115
try {
113116
const data = JSON.parse(e.data as string);
114117
const clientId = data.clientId;
@@ -143,7 +146,7 @@ function subscribeToPocketbaseSSE(
143146
}
144147
};
145148

146-
const onError = (error: any) => {
149+
const onError = (error: EventSourceErrorEvent) => {
147150
if (stabilityTimer) {
148151
clearTimeout(stabilityTimer);
149152
stabilityTimer = null;
@@ -153,12 +156,17 @@ function subscribeToPocketbaseSSE(
153156
baseUrl,
154157
});
155158

156-
const normalizedError = new Error(
157-
(error && error.message) || "PocketBase SSE connection failure",
158-
);
159-
if (error && error.code) (normalizedError as any).code = error.code;
160-
if (error && error.status) (normalizedError as any).status = error.status;
161-
(normalizedError as any).originalErrorEvent = error;
159+
const normalizedError = new Error(error.message || "PocketBase SSE connection failure") as Error & {
160+
code?: number;
161+
status?: number;
162+
originalErrorEvent?: EventSourceErrorEvent;
163+
};
164+
// `status` isn't part of eventsource's declared ErrorEvent shape, but the
165+
// library has been observed attaching it on some HTTP error responses.
166+
const status = (error as unknown as { status?: number }).status;
167+
if (error.code) normalizedError.code = error.code;
168+
if (status) normalizedError.status = status;
169+
normalizedError.originalErrorEvent = error;
162170

163171
// Only emit error on the first failure. Subsequent reconnect attempts will not flood the error stream.
164172
if (consecutiveFailures === 0) {
@@ -168,17 +176,17 @@ function subscribeToPocketbaseSSE(
168176
reconnect();
169177
};
170178

171-
const onMessage = (e: any) => {
179+
const onMessage = (e: MessageEvent) => {
172180
try {
173181
const data = JSON.parse(e.data as string);
174182
if (events.includes(data.action) && data.record) {
175-
const output = {
183+
const output: IDataObject & { __original_action?: unknown; __action?: unknown } = {
176184
...data.record,
177185
};
178186
if ("action" in output) {
179-
(output as any).__original_action = output.action;
187+
output.__original_action = output.action;
180188
}
181-
(output as any).__action = data.action;
189+
output.__action = data.action;
182190

183191
this.emit([this.helpers.returnJsonArray(output)]);
184192
}

0 commit comments

Comments
 (0)