Repository navigation
Conversation
Delegate to Promise.finally() so cleanup callbacks do not replace results or turn processing errors into fulfilled promises. Signed-off-by: Don Yin <Don_Yin@outlook.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: preserving native Promise semantics in result finally() methods. |
| Docstring Coverage | |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3 files.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
LazyResult.finally()andNoWorkResult.finally()currently usethen(cb, cb). A cleanup callback's return value replaces theResult, and a rejected processing promise becomes fulfilled if cleanup succeeds.For example:
This delegates to the underlying promise's
finally()to follow the standard Promise API documented in the types. NativePromise.finally()is available throughout the supported Node.js range, so no compatibility helper is needed.The six regressions extend
processor.test.tsand cover no plugins, synchronous plugins and asynchronous plugins. They check result/error identity, a callback with no arguments, and completion of asynchronous cleanup. All six fail ond1908579and pass with the change.Validation:
env -u NO_COLOR pnpm testpasses (727 tests, 100% line coverage, lint, declarations, version, integration and size checks);pnpm exec tsc --noEmitalso passes. Windows and the other Node.js versions were not run locally.