docs: warn about empty-object UwsSocketImpl.data cast at class level (#191)#200
Conversation
…OSSFORGE#191) `UwsSocketImpl` initializes `_data = {} as TData` in its constructor. That cast is safe for object `TData` without required fields, but it silently lies for primitive `TData` or types with required properties: `socket.data` then reads as `{}` typed at `TData`, and the caller is none the wiser. The `data` getter already mentions this, but readers browsing the class for an API overview can miss the per-property note. Lift the warning to the class-level JSDoc so it shows up in IntelliSense and the rendered API docs alongside the rest of the class description.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Thanks for merging, @VikramAditya33. The class-level warning about the empty-object data cast should save users a confusing runtime surprise. |
Summary
UwsSocketImplinitializes_data = {} as TDatain its constructor. That cast is safe for objectTDatawithout required fields (the common case), but it silently lies for primitiveTDataor types with required properties:socket.datathen reads as{}typed atTData, and the caller is none the wiser.The
datagetter already mentions this caveat, but readers browsing the class for an API overview easily miss the per-property note. Lift the warning to the class-level JSDoc so it shows up in IntelliSense, generated API docs, and the rendered class documentation alongside the rest of the description.Changes
src/websocket/core/socket.ts: extend theUwsSocketImplclass JSDoc with the "setsocket.databefore reading it for primitive / required-fieldTData" caveat lifted from thedatagetter's existing note.Closes #191