feat: accept awaitable factory #377
Conversation
WalkthroughThe pull request introduces modifications to the Tataku library's error handling and type definitions. The changes primarily focus on enhancing asynchronous support for factory functions and improving error reporting. A new utility function Changes
Sequence DiagramsequenceDiagram
participant User
participant prepareStreams
participant Factory
participant convertError
User->>prepareStreams: Call with recipe
prepareStreams->>Factory: Load components
Factory-->>prepareStreams: Return results/promises
prepareStreams->>convertError: Convert potential errors
convertError-->>prepareStreams: Standardized errors
prepareStreams->>User: Return streams or error
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
denops/tataku/utils.ts (1)
46-52: Consider incorporating the cause into the error messageWhile passing
causein the error’s options is good, you might also consider dynamically including the cause (if it's a string or other meaningful value) directly in the message for improved diagnostic clarity.Example diff showing a possible approach:
return new Error( - message, + typeof cause === "string" ? `${message}: ${cause}` : message, { cause } );denops/tataku/tataku.ts (3)
46-49: Eliminate redundantPromise.resolveusageSince
factory(...)may return either a Promise or a synchronous value,ResultAsync.fromPromisecan handle it. CallingPromise.resolve(...)is not strictly necessary and can be simplified to improve readability.- ResultAsync.fromPromise( - Promise.resolve(factory(denops, recipe.collector.options ?? {})), - convertError("Failed to load collector"), - ) + ResultAsync.fromPromise( + factory(denops, recipe.collector.options ?? {}), + convertError("Failed to load collector"), + )
55-58: Remove extraPromise.resolvecallSame rationale as above. You can pass
factory(denops, page.options ?? {})directly intofromPromise.- ResultAsync.fromPromise( - Promise.resolve(factory(denops, page.options ?? {})), - convertError("Failed to load processor"), - ) + ResultAsync.fromPromise( + factory(denops, page.options ?? {}), + convertError("Failed to load processor"), + )
65-68: Simplify promise conversionLikewise, the explicit
Promise.resolveis redundant. Remove it for consistency with how you're handling the collector and processor.- ResultAsync.fromPromise( - Promise.resolve(factory(denops, recipe.emitter.options ?? {})), - convertError("Failed to load emitter"), - ) + ResultAsync.fromPromise( + factory(denops, recipe.emitter.options ?? {}), + convertError("Failed to load emitter"), + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
denops/tataku/load.ts(3 hunks)denops/tataku/tataku.ts(2 hunks)denops/tataku/utils.ts(1 hunks)
🔇 Additional comments (3)
denops/tataku/load.ts (3)
16-16: Great addition for async supportAllowing factories to return either a synchronous or asynchronous result is a solid approach, making it more flexible for loading configurations or resources on demand.
94-94: Enhanced error messageThis change clarifies the nature of the failure when loading the processor, which is helpful in debugging.
113-113: Consistent and clear error messageUpdating the emitter load error message to match the same style of clarity is a nice improvement, ensuring consistency across the codebase.
Some provider needs to load config file from other resource as
Promise<T>.So we need change
Factoryto acceptable forPromise.Summary by CodeRabbit
New Features
convertErrorutility functionBug Fixes
Refactor