feat/typing#34
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces improved TypeScript typing for workflow services by extracting only public parts of service classes and supporting PromiseLike return types in addition to Promise types.
Changes:
- Added
PublicPartutility type to map services to their public interfaces only - Extended
OnlyAsynctype to support bothPromiseandPromiseLikereturn types - Updated
.npmignorepatterns for better build artifact exclusion
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/PublicPart.ts | New utility type to extract public members from service types |
| src/workflows/index.ts | Applied PublicPart typing to service parameters in workflow run methods |
| src/types/OnlyAsync.ts | Extended type check to include PromiseLike alongside Promise |
| src/tests/workflows/public-part.ts | Test case demonstrating public-only service interface usage |
| src/tests/workflows/promise-like.ts | Test case for PromiseLike return type support |
| src/tests/workflows.test.ts | Added test assertions for new public-part and promise-like workflows |
| package.json | Version bump to 0.6.1 |
| .npmignore | Updated ignore patterns for debug files and eslint configurations |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,4 @@ | |||
| /** | |||
| * @internal | |||
There was a problem hiding this comment.
The PublicPart type implementation is identical to TypeScript's built-in mapped type and doesn't actually filter out private members. TypeScript's type system doesn't have runtime visibility into public/private modifiers, so this type provides no actual filtering functionality. Consider renaming to reflect its actual behavior (e.g., ServiceInterface) or documenting that it serves as a semantic marker for service interfaces rather than an enforcer of visibility.
| * @internal | |
| * @internal | |
| * | |
| * Semantic alias for the "public interface" of a type. | |
| * | |
| * Note: This type does **not** filter out private or protected members. | |
| * TypeScript's type system has no runtime notion of visibility modifiers, | |
| * so this is effectively an identity mapped type: | |
| * | |
| * { [K in keyof T]: T[K] } | |
| * | |
| * Use this as a documentation aid to indicate that a type represents | |
| * the externally visible shape of a service or API, not as an enforcer | |
| * of public/private boundaries. |
| // Assert | ||
| assert.ok(result); | ||
| assert.ok(result.id); | ||
| assert.equal(result.result, 3); |
There was a problem hiding this comment.
The test expects result to be 3, but the mock implementation in the test only adds x + y (1 + 2 = 3), while the actual MathService.add method includes + await this.z() which returns 1, making the real result 4. This test passes with incorrect expectations because the mock doesn't match the actual service behavior.
No description provided.