Conversation
|
@Teages is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
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 (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a TaskConcurrency type and optional ChangesTask Concurrency Support
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/runtime/internal/task.ts (1)
62-109: ⚡ Quick winMove internal helper functions to the end of the file.
Lines 62-109 introduce non-exported helpers before later exported APIs. Please move these helpers below exported declarations to match project structure conventions.
As per coding guidelines, “Place non-exported/internal helpers at the end of the file.”
🤖 Prompt for 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. In `@src/runtime/internal/task.ts` around lines 62 - 109, The four internal helper functions _callTask, _getTaskConcurrencyKey, _runTaskOnce, and _runTaskSerially (which reference __runningTasks__ and __serialQueues__) are defined before exported APIs; move these non-exported helpers to the end of the file after all exported declarations so they live as internal utilities per project convention, preserving their implementations and imports/closures exactly as-is to avoid changing behavior.
🤖 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 `@src/runtime/internal/task.ts`:
- Around line 49-60: The current task dispatcher treats any concurrency.mode
value other than "parallel" or "serial" as dedupe, silently accepting invalid
configs; update the logic in the function containing concurrency.mode (which
calls _callTask, _getTaskConcurrencyKey, _runTaskSerially, and _runTaskOnce) to
explicitly handle known modes and throw a clear Error for unknown
concurrency.mode values instead of falling through to _runTaskOnce, so invalid
runtime configuration surfaces immediately.
---
Nitpick comments:
In `@src/runtime/internal/task.ts`:
- Around line 62-109: The four internal helper functions _callTask,
_getTaskConcurrencyKey, _runTaskOnce, and _runTaskSerially (which reference
__runningTasks__ and __serialQueues__) are defined before exported APIs; move
these non-exported helpers to the end of the file after all exported
declarations so they live as internal utilities per project convention,
preserving their implementations and imports/closures exactly as-is to avoid
changing behavior.
🪄 Autofix (Beta)
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: bcb5d5cf-826b-4b19-ae17-e53f7a0e72e6
📒 Files selected for processing (4)
docs/1.docs/50.tasks.mdsrc/runtime/internal/task.tssrc/types/runtime/task.tstest/unit/task-concurrency.test.ts
|
Thanks for PR. This is nice to add control but i suggest to keep it simple. Main thing we should do (by default) is to hash task params and make sure name+params is unique (i made pr draft to update it) |
|
This stems from our actual needs: some data synchronization tasks can be dedupeed, but some update tasks should run in serial. A custom key function is also necessary: I usually add some extra parameters to params, which only used in logging. @pi0 I have update the default key to |
commit: |
|
Rebased this on latest main locally. Only the docs concurrency section conflicted, and it was easy to resolve.
This comment was written by an AI assistant on behalf of the Nitro maintainers. Please double-check anything that looks wrong. |
🔗 Linked issue
#1974
❓ Type of change
📚 Description
The PR added a
concurrencyoption to task, allow developer to control task concurrency behavior.By default it is
dedupeandkey: (event) => ohash(event.payload).📝 Checklist