-
Notifications
You must be signed in to change notification settings - Fork 7.8k
V1 component types #11305
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
amanape
wants to merge
17
commits into
main
Choose a base branch
from
APP-47/v1-component-types
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
V1 component types #11305
+391
−484
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Implement basic useWebSocket hook with connection management - Add comprehensive test suite using MSW for WebSocket mocking - Support connection establishment, message handling, and error management - Include state management for isConnected, messages, lastMessage, and error - Handle both connection closure errors (onclose) and protocol errors (onerror) - Follow TDD principles with failing tests first, then minimal implementation Tests implemented: - ✅ should establish a WebSocket connection - ✅ should handle incoming messages correctly - ✅ should handle connection errors gracefully - 🔄 should close the WebSocket connection on unmount (todo) - 🔄 should close the WebSocket when called explicitly (todo) Co-authored-by: openhands <[email protected]>
- Convert function declaration to arrow function syntax for consistency - Remove unused todo test for explicit close functionality - All 4 tests still passing after refactor: ✅ should establish a WebSocket connection ✅ should handle incoming messages correctly ✅ should handle connection errors gracefully ✅ should close the WebSocket connection on unmount Co-authored-by: openhands <[email protected]>
- Add optional second parameter for query parameters configuration - Support Record<string, string> type for query parameters - Use URLSearchParams for proper URL encoding and building - Maintain full backward compatibility with existing usage - Update useEffect dependencies to track options changes Usage: useWebSocket('ws://example.com/ws', { queryParams: { token: 'abc123', userId: 'user456' } }) // Results in: ws://example.com/ws?token=abc123&userId=user456 Tests: ✅ 5/5 passing - ✅ should establish a WebSocket connection - ✅ should handle incoming messages correctly - ✅ should handle connection errors gracefully - ✅ should close the WebSocket connection on unmount - ✅ should support query parameters in WebSocket URL Co-authored-by: openhands <[email protected]>
- Add onOpen, onClose, onMessage, and onError callback options - Implement TDD approach with comprehensive test coverage - Support custom event handlers while maintaining existing functionality - Call onError for both onerror events and error closures (non-1000 codes) - All tests passing with MSW WebSocket mocking Co-authored-by: openhands <[email protected]>
- Add sendMessage function to send data through WebSocket connection - Implement proper connection state checking (only send when OPEN) - Support all WebSocket data types (string, ArrayBufferLike, Blob, ArrayBufferView) - Add comprehensive test coverage for both connected and disconnected states - Use React.useCallback for performance optimization - Follow TDD approach with failing tests first Co-authored-by: openhands <[email protected]>
Co-authored-by: openhands <[email protected]>
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like
Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
End-user friendly description of the problem this fixes or functionality this introduces.
Summarize what the PR does, explaining any non-trivial design decisions.
Link of any specific issues this addresses:
To run this PR locally, use the following command:
GUI with Docker:
CLI with uvx: