Skip to content

Fix setTimeout and add npm publish CI - #5

Merged
rafalzawadzki merged 1 commit into
masterfrom
rafalzawadzki/fix-settimeout
Feb 24, 2026
Merged

rafalzawadzki merged 1 commit into
masterfrom
rafalzawadzki/fix-settimeout

Conversation

@rafalzawadzki

Copy link
Copy Markdown

Summary

  • Replace restricted setTimeout with sleep from n8n-workflow (fixes community package scanner violation)
  • Add no-restricted-globals eslint rule to prevent future use of setTimeout/setInterval/setImmediate
  • Add GitHub Actions workflow to publish to npm with --provenance on merge to master (using Trusted Publishers)
  • Version bump to 0.5.1

Test plan

  • pnpm build passes
  • pnpm lint passes with no restricted globals violations
  • Merge to master and verify the publish workflow runs successfully on GitHub Actions

🤖 Generated with Claude Code

- Replace restricted setTimeout with sleep from n8n-workflow
- Add no-restricted-globals eslint rule to catch future violations
- Add GitHub Actions workflow for npm publish with provenance on merge to master
- Bump version to 0.5.1

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude

claude Bot commented Feb 24, 2026 •

Copy link
Copy Markdown

Claude finished @rafalzawadzki's task —— View job


Pull Request Review

I've completed my review of this PR. Overall, this is a well-executed fix that properly addresses the n8n community package requirements. Here's my detailed feedback:

Todo List

  • Read repository guidelines (CLAUDE.md) - No CLAUDE.md found
  • Review setTimeout → sleep change in GenericFunctions.ts
  • Analyze eslint rule addition
  • Review GitHub Actions publish workflow security
  • Check package.json version bump
  • Provide comprehensive feedback

✅ Strengths

1. Correct setTimeout Replacement - nodes/Supadata/GenericFunctions.ts:109

  • Properly imported sleep from n8n-workflow
  • Correctly replaced setTimeout with await sleep(pollIntervalMs)
  • Maintains the same polling behavior while using n8n-approved APIs

2. Proactive Prevention - .eslintrc.js:49

  • Added no-restricted-globals rule to prevent future violations
  • Comprehensive coverage of all restricted globals: setTimeout, setInterval, setImmediate
  • Applied specifically to nodes directory where it matters most

3. Secure CI/CD Setup - .github/workflows/publish.yml

  • Uses minimal required permissions (contents: read, id-token: write)
  • Implements npm provenance with --provenance flag for supply chain security
  • Proper dependency caching and frozen lockfile usage
  • Includes build and lint verification before publishing

🔧 Minor Recommendations

1. Workflow Optimization - .github/workflows/publish.yml:30
Consider adding --access public to the publish command if this is intended to be a public package:

- run: pnpm publish --provenance --no-git-checks --access public

2. Error Handling Enhancement - nodes/Supadata/GenericFunctions.ts:109
The polling logic looks robust, but consider logging polling attempts for debugging:

// Optional: Add debug logging
console.log(`Polling job ${jobId}, attempt ${Math.floor((Date.now() - startTime) / pollIntervalMs) + 1}`);
await sleep(pollIntervalMs);

3. Workflow Testing
The publish workflow should be tested in a fork or with a dry-run before the first real publish to ensure npm credentials and provenance work correctly.

📊 Code Quality Assessment

  • Security: ✅ Excellent - Proper use of n8n APIs, secure CI/CD setup
  • Performance: ✅ Good - Sleep implementation is more efficient than setTimeout for n8n
  • Maintainability: ✅ Excellent - ESLint rules prevent regression
  • Test Coverage: ⚠️ Not evaluated - No test changes visible, but this is primarily a dependency compliance fix

🔍 No Issues Found

  • No security vulnerabilities detected
  • No performance regressions
  • No breaking changes to public APIs
  • Proper version bumping (0.5.0 → 0.5.1)

📋 Final Recommendation

APPROVE - This PR successfully addresses the n8n community package scanner violation while implementing best practices for prevention and CI/CD. The changes are minimal, focused, and well-executed.

The only remaining test item mentioned in the PR description is verifying the publish workflow runs successfully on merge to master, which is appropriate to test after merge.


@rafalzawadzki
rafalzawadzki merged commit 5109099 into master Feb 24, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant