fix(types): add StreamConfig interface and update StreamState type alias - #389
fix(types): add StreamConfig interface and update StreamState type alias#389Songu3020 wants to merge 5 commits into
Conversation
|
@Songu3020 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
…ases - Implement advanced scenario tests for StreamBuilder including lifecycle management, validation of ratePerSecond, amount, and address formats. - Add tests for ConduitBatcher covering payload validation, chunking behavior, and lifecycle management. - Introduce edge case tests for fee estimator, signer, and Soroban interactions, ensuring robust error handling and state management. - Enhance StreamsModule tests to cover various edge cases, including transaction handling, simulation failures, and validation checks. - Create tests for Soroban caching and invocation, ensuring correct behavior under different scenarios.
- Improved `toStroops` and `fromStroops` functions with memoization for faster conversions. - Updated `StreamBuilder` to use a Set for `pendingQueue`, optimizing queue operations. - Refactored `subscribeToStream` to utilize a shared server cache for event polling. - Added comprehensive tests for `GraphQLIndexer`, covering constructor validation, query execution, subscription management, and error handling. - Enhanced documentation to reflect performance improvements and architectural changes.
Jaydbrown
left a comment
Jaydbrown
left a comment
There was a problem hiding this comment.
Can't merge this — the diff doesn't match the description at all.
All 65 changed files are generated coverage/lcov-report/** HTML/JS report output (base.css, prettify.js, per-file .ts.html pages, etc.) — nothing in src/. There's no StreamConfig/StreamState/Recipient export change in src/index.ts anywhere in this diff, despite that being exactly what the PR description says it does. This looks like a local coverage/ directory got committed by accident (e.g. via git add -A after running the test-coverage command) instead of the actual intended source change.
Also, the linked issue (#376) is itself an unrelated placeholder ("Detailed enhancement request #42... improve performance by 20%... 90% coverage") that doesn't match either the PR description or the diff — worth double-checking the right issue is referenced once this is fixed.
To fix: drop the coverage/ files from this branch (and consider adding coverage/ to .gitignore so this can't happen again — it isn't currently ignored), then push the real change: adding export type { StreamConfig, StreamState, Recipient } from './types.js'; (or wherever these are actually defined) to src/index.ts. Happy to review once the branch has the real diff.
2 participants
#closes
#376
Problem
Users are manually defining types for things like StreamConfig because the SDK doesn't export them from the root index.
Solution
Ensure export type { StreamConfig, StreamState, Recipient } from './types' is present in src/index.ts.