fix: lint errors and prepare package for release - #78
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates Hostinger node metadata, extracts shared resource options, normalizes Docker and Reach labels, improves request-body error context, and updates package release and tooling configuration. ChangesHostinger node updates
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
nodes/hostingerApi/HostingerApi.node.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🔇 Additional comments (6)
nodes/hostingerApi/HostingerApi.node.ts (1)
9-38: LGTM!Also applies to: 67-67, 92-101, 408-420, 435-438, 453-459, 474-481
nodes/hostingerApi/HostingerApi.node.json (2)
2-2: 🎯 Functional CorrectnessVerify the community-node codex identifier format.
The new value includes
.HostingerApi. The current n8n community-node starter uses the package name alone in this field, while the node implementation uses the runtime namehostingerApi. Confirm the schema required by@n8n/node-cli@0.43.2and use the exact identifier expected by codex lookup. (raw.githubusercontent.com)
3-6: LGTM!Also applies to: 20-20
package.json (3)
41-41: 🩺 Stability & Availability
⚠️ Unverified finding
Sandbox verification was unavailable.Verify that
@n8n/node-cli@0.43.2is published.The npm package page currently reports
0.40.3as the latest release. Confirm that0.43.2exists in the registry before merging. If it does not exist, clean installs will fail. (npmjs.com)Verify the registry version
3-3: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
⚠️ Unverified finding
Sandbox verification was unavailable.Commit the lockfile for these manifest changes.
The tracked
package-lock.jsonstill declares version1.0.22and@n8n/node-cli^0.23.0, while this manifest declares1.0.23and^0.43.2.npm ciexits when dependency declarations do not match the lockfile. Regenerate and commitpackage-lock.json. (raw.githubusercontent.com)Verify manifest and lockfile metadata
Also applies to: 41-48
41-48: 🔒 Security & PrivacyAudit the dependency tree after removing
overrides.Removing these overrides changes transitive versions used by the build and lint toolchain. Regenerate the lockfile, run the dependency audit, and confirm that repository-level Dependabot dismissals cover only the intended tooling findings before release.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/hostingerApi/HostingerApi.node.ts`:
- Around line 1954-1957: Update the request-body handling around the
NodeOperationError branch to parse and validate requestBody before issuing the
HTTP request, using parseJsonParam('requestBody', 'Request Body') or converting
JSON.parse SyntaxError into the expected error type. Ensure malformed JSON
follows the existing continueOnFail behavior for both settings and prevents the
HTTP request; add coverage for both cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b28034c9-2e9a-4964-a91f-6ef225088abb
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
nodes/hostingerApi/HostingerApi.node.jsonnodes/hostingerApi/HostingerApi.node.tspackage.json
Fix n8n community node review blockers (v1.0.23)
v1.0.22 was rejected by n8n's automated review, so the update never reached the n8n marketplace (v1.0.21 remains live for users). This fixes all 45 linter violations plus both manual findings from the review.
Why local lint passed while the review failed
Two independent reasons, both fixed here:
@n8n/scan-community-packageruns eslint with--no-inline-config, so the 43eslint-disable-next-linecomments inHostingerApi.node.tswere ignored. Suppressions can no longer hide these violations, they had to be fixed for real.@n8n/node-cliwas pinned at^0.23.0, which predates therequire-node-api-errorandno-overrides-fieldrules entirely. Bumped to^0.43.2, sonpm run lint(which CI runs before publish) now catches these before submission instead of after.Changes
nodes/hostingerApi/HostingerApi.node.tsactionstrings via the rule's own autofix (node-param-operation-option-action-miscased). Note this lowercases brand words:'Create Docker project'→'Create docker project','List Reach contacts'→'List reach contacts'. Only ALL-CAPS words are exempt from the rule, which is whyVPS/DNSactions were never flagged.resourceOptionsconst, satisfyingnode-param-resource-with-plural-option. The rule's autofix would renameDNS→DNand thenVPS→VP, sincepluralizetreats a trailing "S" as plural. The rule intentionally skips options that point to an identifier, and this is also the layout n8n's own nodes use for their descriptions.throw new NodeOperationError(this.getNode(), error, { itemIndex: i }), matching the wrapping already used for HTTP failures (require-node-api-error). The rule flagsthrow <catchParam>even when the value is already aNodeOperationError.eslint-disable-next-linecomments.nodes/hostingerApi/HostingerApi.node.json(codex, both manual findings)node:n8n-nodes-base.HostingerApi→n8n-nodes-hostinger-api.HostingerApi(package prefix must match the npm package name).categories:API→Development(APIis not in n8n's documented set).package.jsonoverridesfield, see the section below.@n8n/node-cli:^0.23.0→^0.43.2.1.0.23. CI publishes whatever version is inpackage.json, and publishing a new version is what re-triggers n8n's review; no manual resubmission is needed.Removal of
overrides- required, and safeThe linter rejects the field outright:
Those overrides were deliberate Dependabot fixes (#62, #63, #65, #67, #74), so to be explicit about what is and isn't lost:
dist/only, declares zero runtimedependencies, andn8n-workflowis a peer supplied by n8n itself. Every package the overrides targeted lives exclusively in the build tree, so no consumer ever resolved them, which is precisely the linter's argument.package-lock.json. After removal,npm auditreports 13 entries (8 high, 5 moderate) stemming from just 2 advisories:nanoid(high, via@n8n/utils) anduuid@10(moderate, via@langchain/classic/@langchain/community). Both arrive through@n8n/node-cli→@n8n/ai-node-sdk; the remaining 11 entries are transitive "depends on vulnerable versions of" rollups.Alternatives considered and rejected:
devDependenciesinstead: does not work.@n8n/utilsrequiresnanoid@^3, so npm nests the vulnerable copy regardless.overridesis the only npm mechanism that forces transitive versions.Action required before merge: suppress the dev-tree Dependabot alerts
@zygintas - this needs one manual step, and it should be done before this PR merges: auto-triage rules are applied as alerts arrive, so creating the rule first means these are never opened at all. (There are 0 open alerts on
maintoday only because the overrides are still in place.)
Repository Settings → Advanced Security → Dependabot rules → New rule:
npm; package namesnanoid,uuid,lodash,form-data,flatted,langsmith,@langchain/communityJustification to record in the rule description: the published package contains
dist/only with zero runtimedependencies; all matching packages are reachable solely through@n8n/node-cli(build/lint tooling) or then8n-workflowpeer that n8n itself provides, so they are never installed by an n8n user.