Conversation
WalkthroughThis pull request introduces asynchronous callback support for the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ConstraintFunction
participant AsyncCallback
Caller->>ConstraintFunction: Invoke constraint validation
ConstraintFunction->>ConstraintFunction: Iterate constraints using for...of
ConstraintFunction->>AsyncCallback: await cb(value, ...)
AsyncCallback-->>ConstraintFunction: Return value or promise resolution
ConstraintFunction->>Caller: Return aggregated results
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
plugins/constraints/src/external.constraint.ts (1)
38-41: Good refactoring from forEach to for...of loops to support await.The loop structure has been properly changed to enable awaiting asynchronous callbacks. This ensures each callback is fully processed before moving to the next one, maintaining proper execution order.
Consider adding documentation comments mentioning that async callbacks are now supported, which would make the API more self-documenting.
/** + * @param validator - The name of the validator to apply + * @param cb - The callback function to execute, which can now be async */Also applies to: 50-51
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/healthy-donuts-invite.md(1 hunks)plugins/constraints/src/external.constraint.ts(3 hunks)plugins/constraints/src/external.sheet.constraint.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Setup
🔇 Additional comments (5)
plugins/constraints/src/external.constraint.ts (1)
20-20: Correctly updated return type to support async callbacks.The return type now includes
Promise<any>, allowing for asynchronous callback functions..changeset/healthy-donuts-invite.md (1)
1-6: Changeset message is clear and concise.The changeset correctly identifies this as a patch update for the
@flatfile/plugin-constraintspackage and clearly states the purpose: supporting async callbacks.plugins/constraints/src/external.sheet.constraint.ts (3)
29-29: Correctly updated function signature to support async callbacks.The return type now properly includes
Promise<any>, which aligns with the PR objective of adding async callback support.
47-63: Good refactoring to support async operations.Converting from
forEachtofor...ofloops is essential for proper async execution, as it allows the use ofawaitwithin the loop. This change properly ensures that:
- Each constraint is processed sequentially
- Each record within a constraint is processed sequentially
- The callback is properly awaited with
await cb(...)The error handling is preserved and works correctly with the async pattern.
51-51: Correctly implemented await for callback execution.Adding
awaithere ensures that the callback is properly awaited, whether it returns a direct value or a Promise.
meritmalling
left a comment
There was a problem hiding this comment.
Tested & works great!
Please explain how to summarize this PR for the Changelog:
This adds support for async
cb()Tell code reviewer how and what to test: