Repository navigation
docs: tidy code comments in package/src - #3884
Merged
Merged
Conversation
Rewrite the code and doc comments of the SDK source and ai-components in the present tense and keep only what is non-obvious. Fix comments that disagreed with the code, add missing @param/@returns tags to documented public functions, and remove commented-out code. No behavior change.
oliverlaz
requested review from
isekovanic and
szuperaz
and removed request for
isekovanic
October 21, 2026 05:58
The errors thrown by the `useXContext()` hooks linked to the removed `basics/hello_stream_chat` page, which now returns 404. Link to the current core-components / ui-components pages instead, and name the component that actually provides each context: - `useAttachmentPickerContext`: Channel (provided in ChannelShell), not OverlayProvider - `useMessageListItemContext`: MessageList (no `MessageListItem` component exists) - `useCreatePollContentContext`: CreatePoll, which wraps CreatePollContent in the provider Also assert the messages of the hooks the suite did not cover.
Contributor
SDK Size
|
1 of 6 tasks
isekovanic
added a commit
that referenced
this pull request
Oct 21, 2026
## 🎯 Goal Fix the code issues and wrong strings found while tidying the comments in #3884, which deliberately left them alone (comment-only change). **Stacked on #3884**: merge that first, then retarget this PR to `V10`. ## 🛠 Implementation details Every item was verified against the code first; the behavior fixes each come with a test that failed before the fix. **Bugs fixed** - **Markdown links** (`generateMarkdownText`): the conversion loop restarted from the untrimmed text on every link, so only the last link became a markdown link and the trim was lost. Links are now converted in one pass, longest first, so `getstream.io` inside `getstream.io/chat` is not wrapped twice. - **Offline drafts** (`upsertDraft`): the result of `queries.concat(...)` was discarded, so a draft's quoted and parent messages were never stored, although `getDraft` reads the quoted message back from `messages`. - **AI markdown `onLink`**: a rejected `onLink` was logged and rethrown inside a fire-and-forget `.catch`, causing an unhandled rejection. Now only logged. - **`parseJsonChart`**: invalid JSON threw a `SyntaxError` instead of the documented "Unknown type" error. - **`MessageActionList`**: destructive actions were given `length: standardActions.length` (visible to custom `MessageActionListItem` overrides). - **Emoji middleware**: an emoji with no `native` and empty `skins` threw a `TypeError`; such entries are now skipped (there is nothing to insert). - **`FileIcon`**: `.xlsb` used the `.xlam` MIME type and so showed the generic icon; removed duplicate `.odt/.ott/.fodt`, `application/gzip` and `video/webm` entries. - Removed an unreachable `null` branch in `getReactionsForFilterSort` and a dead `?? 0` in `hashStringToNumber`. **Strings** - **Permission alerts**: the shared `common.cameraPermission.text` / `common.galleryPermission.text` copy was ungrammatical ("Device gallery permissions is used to take photos or videos") and is fixed. The image-only channel image flow gets its own keys, `channelDetails.editImageSheet.cameraPermission.text` / `.galleryPermission.text`, instead of saying "photos or videos". - **`useMessagesContext` error**: named `MessageList` (the context comes from `<Channel>` / `<Thread>`) and linked a docs page that now returns 404. It now names `Channel` and links the live Channel page (the other context errors are fixed in #3884). - Clearer warning when the attachment picker cannot open (it reports the missing `snapToIndex`), an accurate label for the gallery's video placeholder, and fixed docs for `previewVoiceRecording`, the `useIsMountedRef` example and an `eslint-disable` description. - `keys.ts` regenerated; the SampleApp German and Italian dictionaries updated for the new and changed keys. **⚠️ Breaking: removed props that were never read** (v10 is a breaking release; listed in `ai-docs/v10-breaking-changes.md`) - `ProgressControl`: `width` (the control measures its own width). - `MessageActionList`: `dismissOverlay` (each item closes the overlay itself). A custom list that needs it can read `useMessageContext().dismissOverlay`. - `MessageFlashList`: `inverted` (the FlashList is never inverted). **Checked and left unchanged** - **Live locations in `upsertChannels`**: the `forEach(async)` really does drop the queries (they are never written today). A naive fix would make it worse: `locations.messageId` has a foreign key to `messages`, so a live location whose message is outside the stored window fails the whole channel batch. This needs a decision (drop the foreign key with a `dbVersion` bump, or write only locations whose message is stored). - **Gallery gestures**: the pan `finalXPosition` sign (velocity-gated paging, chosen in #3241) and the double-tap zoom-in `translateX` stay as they are. - Not bugs: `useRAFCoalescedValue` (a test now guards the re-enable case), the duration units (the gallery and media items both pass milliseconds), the pinch `scale < 1` branches (reachable on iOS) and the read of `translateY.value` right after starting an animation (Reanimated still returns the release position at that point). - `asyncMessagesMinimumPressDuration` is read by `AudioRecordingButton`, so it stays. ## 🎨 UI Changes Only the permission alert copy. ## 🧪 Testing - New or extended tests for every behavior fix: `upsertDraft` (real SQLite), `generateMarkdownText`, `Markdown` (`onLink` rejection), `charts`, `FileIcon`, `emojiControl`, `getTrimmedAttachmentTitle`, `useRAFCoalescedValue`, `ImageGallery`, `ImageGalleryVideoControl`, `MessageActionList`, `useEditChannelImage`, `filePickers`, `useAttachmentPickerBottomSheet` and the context error messages. - `yarn lint`, `yarn typecheck` (all workspaces), `yarn test:unit` and `yarn build` pass. ## ☑️ Checklist - [ ] I have signed the [Stream CLA](https://docs.google.com/forms/d/e/1FAIpQLScFKsKkAJI7mhCr7K9rEIOpqIDThrWxuvxnwUq2XkHyG154vQ/viewform) (required) - [ ] PR targets the `develop` branch (stacked on #3884, then `V10`) - [x] Documentation is updated - [ ] New code is tested in main example apps, including all possible scenarios - [ ] SampleApp iOS and Android - [ ] Expo iOS and Android --------- Co-authored-by: Ivan Sekovanikj <ivan.sekovanikj@getstream.io>
Contributor
|
🎉 This PR is included in version 10.0.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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
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.
🎯 Goal
Tidy the code and doc comments in
package/src(SDK source andai-components; tests and mock-builders are out of scope) so they describe what the code does today, keep only what is non-obvious, and stop contradicting the code. This is a comment-only change: no behavior change.🛠 Implementation details
getOfflineDbEncryptionKeysaid a missing flag is "silently ignored" while the code throwsSQLCIPHER_BUILD_MISSING;buildThumbnailGrid's examples were swapped;getDurationLabelFromDurationsaid seconds but takes milliseconds;@returnson the sqlite query helpers saidstringbut they return[sql, params]; thelanguage.*names were credited to@stream-io/i18ninstead ofstream-chat.@param(dot notation for object fields),@returnswhen it returns a value and@throwswhen it throws. Existing tags were kept. Internals (non-exported,private,@internal) are prose only.VictoryChart,SendUp, the vega-lite schema,flatten-nodes,MessageComposer,GiphyChip,theme).TODO/FIXMElines,eslint-disableand@ts-*directives, the lines carrying@deprecated/@internal/@example/@default/@overrideType, and generated code.package/src:package/{native,expo}-package/src/native/NativeAIDictation.tsgot the same comment fix assrc/ai-components/native-specs/NativeAIDictation.ts, becausevalidate-entry-isolationrequires the three copies to be identical.V10(not stacked on another PR). The PR targetsV10, notdevelop.🎨 UI Changes
None.
🧪 Testing
How "comment-only" was verified:
V10. It is identical for all files except three, which are expected consequences of removing commented-out code:VictoryChart.tsxandvega-lite/schema.ts(prettier collapsed thevictory-nativeimport and theMarkNameenum onto one line, dropping a trailing comma) andSendUp.tsx(the empty JSX expression{}that held a commented-out<Circle />).TODO/FIXMElines and directive counts are identical. It was validated with negative controls (changed string, type, operator, deleted line, stray whitespace, JSX text, editedTODO, renamed@deprecated, reworded@ts-*) that it flags.yarn lint,yarn build,yarn typecheckandyarn test:unit(2,209 tests) pass.What reviewers should look for:
types.ts, the React Native 0.87 and TS2589 / TS2883 notes, the iOSUITextViewcaching note, the FlashListonViewableItemsChangednote and the Android pinch-to-pan handoff.Streami18n,i18n/runtimeDefaults.tsand theChatprops are the largest remaining blocks.Not changed here (follow-ups):
@defaulttags that disagree with the code (attachmentPickerBottomSheetHeight,attachmentSelectionBarHeight,allowConcurrentAudioPlayback),@example ./X.mdpointers to files that do not exist, and the code issues noticed while reading.☑️ Checklist
developbranch (targetsV10, see above)