Skip to content

[CLNP-8740] fix(thread): migrate reply list to Collection-based data source#1445

Merged
sf-tyler-jeong merged 11 commits into
mainfrom
fix/CLNP-8740
Jul 23, 2026
Merged

[CLNP-8740] fix(thread): migrate reply list to Collection-based data source#1445
sf-tyler-jeong merged 11 commits into
mainfrom
fix/CLNP-8740

Conversation

@sf-tyler-jeong

Copy link
Copy Markdown
Contributor

What & why

Scrolling a thread's reply list to the top duplicated the oldest reply — the
manual reducer anchored on the parent (not a reply), so hasMorePrev stayed true
and the inclusive prev fetch re-appended the boundary reply with no dedup.

Replace the manual reducer with the core-ts useGroupChannelThreadMessages
collection hook (matching React Native UIKit). The collection owns the list keyed
by messageId, so dedup / hasMorePrev / in-flight guards are structural.

Changes

  • Wire the collection into ThreadProvider; fetchers use loadPrevious/loadNext.
  • Anchor at the latest reply, remount per parent (keyed ThreadManager) so thread
    switches re-init; start at a specific reply when entered from search/deeplink/quote.
  • Port send/update/delete/resend to hook-based actions with cross-module pubSub.
  • Preserve pending/failed local previews (file/voice/MFM) via localUrl fallback.
  • Gate reply-list LOADING on online + resolved parent (no stray/duplicate spinner).
  • Scope the mirrored list to the current parent; drop handlers that double-reflected
    collection-owned state.
  • Public Thread context & props contract unchanged (backward compatible).

Opening a thread and scrolling the reply list to the top duplicated the
oldest reply. Anchoring on the parent message (never part of the reply set)
left hasMorePrev permanently true, and the inclusive prev fetch re-returned
the boundary reply, which was then appended with no messageId dedup.

Replace the manual-reducer thread list with the collection-based
useGroupChannelThreadMessages hook, matching the React Native UIKit. The
collection owns the reply list keyed by messageId, so dedup, hasMorePrev, and
the in-flight fetch guard are handled structurally instead of by hand.

- Wire the collection into ThreadProvider; drive fetchers from loadPrevious /
  loadNext and defer pagination callbacks until the collection state settles.
- Anchor the collection at the latest reply by default and remount the manager
  per entry message (keyed ThreadManager), matching the React Native UIKit, so
  switching threads re-initializes the reply list cleanly; anchor at a specific
  reply's position instead when opened from a non-parent message (search / deep
  link / quote).
- Port send / update / delete / resend to hook-based message actions with
  cross-module pubSub publishing.
- Preserve existing behavior for local file, voice, and multiple-files
  previews on pending and failed messages via the writable localUrl fallback.
- Gate the reply-list LOADING state on an online connection and a resolved
  parent message, so opening offline, going offline->online, and a normal
  online open match the legacy loading sequence (no stray/duplicate spinner).
- Scope the mirrored reply list to the current parent message: skip while the
  parent is unresolved, and keep only its replies plus local pending replies
  that have no parentMessageId yet. Switching threads (e.g. to a reply-less
  message) now clears the previous thread's replies while newly sent replies
  still appear.
- Remove the message-event handlers that double-reflected state now owned by
  the collection.
- Update and extend tests; keep the public Thread context and props contract
  unchanged.

CLNP-8740
@upwind-code-us

upwind-code-us Bot commented Jul 21, 2026

Copy link
Copy Markdown

Upwind Upwind IaC Scan - ℹ️ Scan skipped (no relevant changes)

@upwind-code-us

upwind-code-us Bot commented Jul 21, 2026

Copy link
Copy Markdown

Upwind Upwind Code Scan - ✅ Proceed with Deployment

4 newly introduced vulnerabilities · 116 resolved · 11 total in this PR vs main

🔶 2 High | 🟡 9 Medium


🔶 High · 1 finding
CVE Package Version Fix
CVE-2024-21539 @eslint/plugin-kit 0.2.0 0.2.3

🟡 Medium · 3 findings
CVE Package Version Fix
CVE-2026-49458 dompurify 3.3.2 3.4.6
CVE-2026-49459 dompurify 3.3.2 3.4.6
CVE-2026-49978 dompurify 3.3.2 3.4.7

Resolved · 116 findings
CVE Package Version
CVE-2026-6402 webpack-dev-server 4.15.1
CVE-2024-29180 webpack-dev-middleware 5.3.3
CVE-2024-45801 dompurify 3.0.3
CVE-2026-26996 minimatch 5.1.6
CVE-2024-37890 ws 7.5.9
CVE-2024-37890 ws 8.16.0
CVE-2024-55565 nanoid 3.3.7
CVE-2026-9595 webpack-dev-server 4.15.1
CVE-2026-32141 flatted 3.2.9
CVE-2024-47875 dompurify 3.0.8
CVE-2026-0540 dompurify 3.3.1
CVE-2024-28849 follow-redirects 1.15.5
CVE-2026-27903 minimatch 5.1.6
CVE-2026-26996 minimatch 3.1.2
CVE-2026-41239 dompurify 3.3.1
CVE-2026-49356 @babel/core 7.23.9
CVE-2024-47068 rollup 2.79.1
CVE-2026-33671 picomatch 2.3.1
CVE-2026-6322 fast-uri 3.0.2
CVE-2026-2950 lodash 4.17.21
CVE-2025-69873 ajv 6.12.6
CVE-2025-26791 dompurify 3.0.3
CVE-2026-27903 minimatch 9.0.3
CVE-2026-3449 @tootallnate/once 2.0.0
CVE-2026-33894 node-forge 1.3.1
CVE-2025-64718 js-yaml 4.1.0
CVE-2024-52798 path-to-regexp 0.1.7
CVE-2026-41240 dompurify 3.0.8
CVE-2025-32997 http-proxy-middleware 2.0.6
CVE-2026-33895 node-forge 1.3.1
CVE-2026-33750 brace-expansion 1.1.11
CVE-2024-29041 express 4.18.2
CVE-2026-6321 fast-uri 3.0.2
CVE-2026-27601 underscore 1.12.1
CVE-2026-2391 qs 6.11.0
CVE-2024-47875 dompurify 3.0.3
CVE-2026-4800 lodash 4.17.21
CVE-2025-13465 lodash 4.17.21
CVE-2024-43799 send 0.18.0
CVE-2026-48779 ws 7.5.9
CVE-2024-43788 webpack 5.90.2
CVE-2026-27904 minimatch 9.0.3
CVE-2026-41238 dompurify 3.0.3
CVE-2025-7783 form-data 3.0.1
CVE-2025-68458 webpack 5.90.2
CVE-2025-61140 jsonpath 1.1.1
CVE-2025-64756 glob 10.3.10
CVE-2026-1615 jsonpath 1.1.1
CVE-2026-27904 minimatch 3.1.2
CVE-2026-41240 dompurify 3.0.3
CVE-2024-47764 cookie 0.5.0
CVE-2026-29074 svgo 2.8.0
CVE-2026-33532 yaml 1.10.2
CVE-2026-22029 @remix-run/router 1.6.3
CVE-2025-5889 brace-expansion 1.1.11
CVE-2025-66030 node-forge 1.3.1
CVE-2021-3803 nth-check 1.0.2
CVE-2026-41238 dompurify 3.0.8
CVE-2026-41907 uuid 8.3.2
CVE-2025-30359 webpack-dev-server 4.15.1
CVE-2025-69873 ajv 8.12.0
CVE-2025-27789 @babel/runtime 7.23.9
CVE-2025-15284 qs 6.11.0
CVE-2025-27789 @babel/runtime 7.22.5
CVE-2026-55602 http-proxy-middleware 2.0.6
CVE-2026-53550 js-yaml 4.1.0
CVE-2024-4067 micromatch 4.0.5
CVE-2025-64718 js-yaml 3.14.1
CVE-2024-4068 braces 3.0.2
CVE-2025-12816 node-forge 1.3.1
CVE-2025-68470 react-router 6.13.0
CVE-2026-53550 js-yaml 3.14.1
CVE-2025-32996 http-proxy-middleware 2.0.6
CVE-2025-27789 @babel/helpers 7.23.9
CVE-2026-41239 dompurify 3.0.3
CVE-2024-43800 serve-static 1.15.0
CVE-2026-40181 react-router 6.13.0
CVE-2026-33672 picomatch 2.3.1
CVE-2024-43796 express 4.18.2
CVE-2026-33750 brace-expansion 2.0.1
CVE-2024-21536 http-proxy-middleware 2.0.6
CVE-2024-52011 launch-editor 2.6.1
CVE-2025-30360 webpack-dev-server 4.15.1
CVE-2026-33532 yaml 2.3.4
CVE-2026-27903 minimatch 3.1.2
CVE-2023-44270 postcss 7.0.39
CVE-2026-48779 ws 8.16.0
CVE-2026-12143 form-data 3.0.1
CVE-2025-68157 webpack 5.90.2
CVE-2024-45296 path-to-regexp 0.1.7
CVE-2026-33228 flatted 3.2.9
CVE-2026-27606 rollup 2.79.1
CVE-2026-41240 dompurify 3.3.1
CVE-2026-41305 postcss 8.4.35
CVE-2024-45590 body-parser 1.20.1
CVE-2026-41239 dompurify 3.0.8
CVE-2026-53632 launch-editor 2.6.1
CVE-2024-45801 dompurify 3.0.8
CVE-2026-33891 node-forge 1.3.1
CVE-2024-21537 lilconfig 3.1.0
CVE-2025-7339 on-headers 1.0.2
CVE-2026-45736 ws 8.16.0
CVE-2026-9277 shell-quote 1.8.1
CVE-2026-4867 path-to-regexp 0.1.7
CVE-2025-5889 brace-expansion 2.0.1
CVE-2026-26996 minimatch 9.0.3
CVE-2026-41305 postcss 7.0.39
CVE-2026-33896 node-forge 1.3.1
CVE-2026-27904 minimatch 5.1.6
CVE-2025-66031 node-forge 1.3.1
CVE-2026-3449 @tootallnate/once 1.1.2
CVE-2024-21538 cross-spawn 7.0.3
CVE-2026-34043 serialize-javascript 6.0.2
CVE-2025-26791 dompurify 3.0.8
CVE-2026-41238 dompurify 3.3.1
CVE-2024-33883 ejs 3.1.9

View full analysis in Upwind Console

Scan completed in 15s

Scan history (6 scans)
Commit Scanned at New Resolved Net
edbe0f4 2026-07-21 09:40 UTC +4 -115 -111
8b06060 2026-07-22 03:32 UTC +4 -113 -109
8c8b5d6 2026-07-22 07:31 UTC +3 -115 -112
a1b15e4 2026-07-22 10:57 UTC +4 -113 -109
a3574ce 2026-07-22 11:27 UTC +4 -116 -112
164292d < 2026-07-23 06:08 UTC +4 -116 -112

Last scanned: 164292d · 2026-07-23 06:08 UTC

@netlify

netlify Bot commented Jul 21, 2026

Copy link
Copy Markdown

Deploy Preview for sendbird-uikit-react ready!

Name Link
🔨 Latest commit 164292d
🔍 Latest deploy log https://app.netlify.com/projects/sendbird-uikit-react/deploys/6a61afc6b490800008d07e3e
😎 Deploy Preview https://deploy-preview-1445--sendbird-uikit-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f074585189

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines 275 to +288
useEffect(() => {
if (stores.sdkStore.initialized && config.isOnline) {
initializeThreadFetcher();
}
}, [stores.sdkStore.initialized, config.isOnline, initializeThreadFetcher]);
store?.setState((prev) => ({
...prev,
loadPrevious: threadDataSource.loadPrevious,
loadNext: threadDataSource.loadNext,
resetWithStartingPoint: threadDataSource.resetWithStartingPoint,
dsSendUserMessage: threadDataSource.sendUserMessage,
dsSendFileMessage: threadDataSource.sendFileMessage,
dsSendMultipleFilesMessage: threadDataSource.sendMultipleFilesMessage,
dsUpdateUserMessage: threadDataSource.updateUserMessage,
dsResendMessage: threadDataSource.resendMessage,
dsDeleteMessage: threadDataSource.deleteMessage,
}), true);
}, [store]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Refresh thread data-source actions when switching threads

When a user opens another thread while the <Thread> component stays mounted (for example in the desktop layout), InternalThreadProvider keeps the old store and this effect runs only once for the newly keyed ThreadManager. On that first render the store can still contain the previous parentMessage, so loadPrevious, loadNext, dsSendUserMessage, etc. are captured from a data source bound to the old thread and never replaced; sends and pagination in the newly displayed thread can then operate on the previous thread. Include the data-source methods/current parent in the dependencies or key/reset the provider store when the thread target changes.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

원인: 컬렉션을 parentMessage ?? propsParentMessage로 생성했는데, parentMessage
영속 store(InternalThreadProvider, key 없음)에서 옵니다. keyed ThreadManager가 스레드
전환 시 리마운트되지만, 훅 init(useLayoutEffect)이 useGetParentMessage의 갱신
(useEffect)보다 먼저 실행돼, 그 시점 store엔 아직 이전 스레드의 parent가 남아 있습니다.
게다가 startingPoint가 상수(MAX)라 parent가 갱신돼도 재init되지 않아
loadPrevious/loadNext가 이전 스레드에 고정될 수 있었습니다.

수정: 컬렉션 parent를 props 기반 우선으로 바인딩 —
(parentMessage ?? propsParentMessage)(propsParentMessage ?? parentMessage).
propsParentMessage(= getParentMessageFrom(message))는 현재 message prop에서
파생되므로 항상 현재 스레드의 parent이고 store 지연이 없습니다. props가 null인 드문
경우에만 fetched parent로 폴백합니다.

…itch

The reply collection was created from `parentMessage ?? propsParentMessage`, where
`parentMessage` comes from the persistent Thread store. When switching threads while
`<Thread>` stays mounted (desktop), the keyed ThreadManager remounts and the hook
initializes before useGetParentMessage refreshes the store, so the store still held
the previous thread's parent — and with a constant MAX startingPoint the collection
never re-initialized. loadPrevious/loadNext could then paginate the previous thread.

Prefer the props-derived parent (`propsParentMessage ?? parentMessage`): it is always
the current thread's parent with no store lag, falling back to the fetched parent only
when the message carries no embedded parent.

Addresses the Codex review on #1445.
Pull in the thread reply-list fixes shipped in @sendbird/uikit-tools 0.1.4: the
reqId delete-collision guard (a sent reply no longer disappears) and the loadNext
pagination guard. Also bumps react-uikit-message-template-view (fixed-mode) to 0.1.4.
@danney-chun

Copy link
Copy Markdown
Contributor

Collection Base로 Migration 과정은 괜찮아 보입니다.
다만 Breaking Change 요소들이 있습니다.

  1. allThreadMessages, localThreadMessages 같은 경우는 state에 들어 있는 거라 Open이 되어 있고, 고객 쪽에서도 실제로 직접 받아서 사용할 수 있습니다.

    • localThreadMessages 를 삭제를 하게 되면 고객쪽에서 patch 후 build Error가 발생을 할 수 있습니다.
    • 기존 방식에서는 allThreadMessages/localThreadMessages가 가지고 있는 Message의 형태가 달랐는데, 현재는 하나로 합쳐져 있어서 고객이 display 할때 문제가 발생 할 수 있습니다.(기존 : allThreadMessages: 성공한 메세지, localThreadMessage: 실패/pending 메세지)
  2. 추가 적으로 action event들도 삭제 한 부분들도 고객이 사용을 하고 있다면 build에 문제가 발생을 할 수 있습니다. 그냥 삭제가 아닌 backward를 고려한 방식이 적용이 되어야 할거 같습니다.

@danney-chun

Copy link
Copy Markdown
Contributor

발생한 이슈에 대한 Test code가 없어 보입니다. 현시점에서 UI까지 이용한 테스트는 힘들어 보이기는 하는데, mocking을 이용해서 관련 테스트 코드가 있으면 좋을거 같습니다.

…tions

The Collection migration changed the public useThread() context contract. Restore
backward compatibility (addresses PR #1445 review):

- Split the mirrored reply list back into the legacy shape: allThreadMessages holds
  succeeded (server) messages, localThreadMessages holds pending/failed. The
  collection keeps them in one list, so split by sendingStatus; ThreadList renders
  both again (localThreadMessages in a second pass, as before).
- Re-expose the 19 low-level action creators removed from useThread().actions
  (on*/sendMessage*/initializeThreadList*/get{Prev,Next}Messages* dispatchers) as
  no-op shims with matching signatures, so customer code referencing them still
  type-checks. The collection now owns this state, so they are intentional no-ops.
Cover the issues flagged in the PR #1445 review: the allThreadMessages/localThreadMessages state split (succeeded vs pending/failed), the backward-compat no-op action shims on useThread().actions, and the thread-switch parent binding (3e6fb32). Confirmed the 3e6fb32 test fails on the pre-fix binding (props-first vs stale store).
@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

Collection Base로 Migration 과정은 괜찮아 보입니다. 다만 Breaking Change 요소들이 있습니다.

  1. allThreadMessages, localThreadMessages 같은 경우는 state에 들어 있는 거라 Open이 되어 있고, 고객 쪽에서도 실제로 직접 받아서 사용할 수 있습니다.

    • localThreadMessages 를 삭제를 하게 되면 고객쪽에서 patch 후 build Error가 발생을 할 수 있습니다.
    • 기존 방식에서는 allThreadMessages/localThreadMessages가 가지고 있는 Message의 형태가 달랐는데, 현재는 하나로 합쳐져 있어서 고객이 display 할때 문제가 발생 할 수 있습니다.(기존 : allThreadMessages: 성공한 메세지, localThreadMessage: 실패/pending 메세지)
  2. 추가 적으로 action event들도 삭제 한 부분들도 고객이 사용을 하고 있다면 build에 문제가 발생을 할 수 있습니다. 그냥 삭제가 아닌 backward를 고려한 방식이 적용이 되어야 할거 같습니다.

  1. 상태(allThreadMessages / localThreadMessages) — 기존 계약을 복원했습니다. 컬렉션은
    하나로 합쳐 관리하지만 mirror에서 sendingStatus로 갈라 allThreadMessages=성공(서버),
    localThreadMessages=pending/failed 로 채우고, ThreadList도 기존처럼 두 배열을 각각
    렌더합니다(localThreadMessages 2차 pass). → 두 필드의 타입·의미가 이전과 동일합니다.

  2. 삭제됐던 actions — useThread().actions에서 빠졌던 19개 저수준 dispatcher
    (on*, sendMessage*, initializeThreadList*, get{Prev,Next}Messages*)를 시그니처 그대로
    다시 노출했습니다. 컬렉션이 상태를 소유하므로 type/build 호환용 no-op이며, 파라미터는
    unused-var lint를 피해 noop as (…) => void 캐스트로 타입만 부여했습니다.

감사합니다.

@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

발생한 이슈에 대한 Test code가 없어 보입니다. 현시점에서 UI까지 이용한 테스트는 힘들어 보이기는 하는데, mocking을 이용해서 관련 테스트 코드가 있으면 좋을거 같습니다.

테스트 추가했습니다. 감사합니다!

@@ -262,10 +263,17 @@ export const ThreadManager: React.FC<React.PropsWithChildren<ThreadProviderProps
if (!parentMessage) return;
const parentId = parentMessage.messageId;
const scopedMessages = threadDataSource.messages.filter((m) => m.parentMessageId === parentId || !m.parentMessageId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useGroupChannelThreadMessages에서 넘겨주는 messages는 thread message만 넘어오지 않나요? 여기서 다시 parentMessageId로 filter를 할 필요가 없어 보입니다.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다. event/fetch 경로(onMessagesAdded/Updated/Deleted, init, loadPrevious/loadNext)는 core-ts에서 이미 isThreadedMessage로 parent 필터링되어 넘어오므로, 그 경로만 보면 여기서의 재필터는 중복이 맞습니다.

다만 local send/resend 경로가 예외입니다. sendUserMessage/sendFileMessage/resendMessage 등 송신·재전송 콜백(onPending/onSucceeded/onFailed)은 channelUrl만 확인하고 parent는 보지 않습니다 (updateUserMessage/updateFileMessageisThreadedMessage까지 확인하는데 send/resend만 빠져 있음). 그래서 전역 onBeforeSendUserMessage 등에서 params.parentMessageId를 바꾸는 커스텀 케이스면, 다른 parent의 로컬 에코가 parent 체크 없이 이 리스트에 섞일 수 있습니다.

threadMessages/allThreadMessages가 public state라 미러링 단계에서 방어적으로 parent 스코프 필터를 유지했습니다. 필터는 m.parentMessageId === parentId || !m.parentMessageId로 관대하게 둬서 아직 parentMessageId가 안 박힌 낙관적 pending echo(방금 보낸 답장)를 실수로 가리지 않게 했고, "다른 parentMessageId 메시지는 제외된다" 회귀 테스트(필터 제거 시 red)도 추가했습니다.

우선 이 PR에서는 react 쪽에서 처리하는 것으로 두겠습니다. 근본적으로는 core-ts의 send/resend 게이팅도 update 경로처럼 맞추는 게 더 깔끔하고(그럼 이 필터도 제거 가능) 별도 core-ts 수정으로 다루면 좋을 것 같습니다.

감사합니다!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그래서 전역 onBeforeSendUserMessage 등에서 params.parentMessageId를 바꾸는 커스텀 케이스면, 다른 parent의 로컬 에코가 parent 체크 없이 이 리스트에 섞일 수 있습니다.

이 부분은 다른 Message가 발생할 때, 이게 보일 수 있는 우려가 있어서 Filter를 유지하신 것 같은데, 반대의 경우에 보여야 할 Thread가 안 보이게 되는 현상도 생길 수 있습니다. core-ts에서 local send/resend에 대한 isThread 체크가 없다면, 이 부분을 수정해서 useGroupChannelThreadMessages에서 나오는 Data의 무결성을 보장하고 Uikit에서는 그걸 믿고 display하는 게 맞을 것 같습니다.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀대로 데이터 무결성을 소스(core-ts)에서 보장하는 방향으로 바꿨습니다.

  • core-ts: local send/resend 경로에 parent 스코프 체크(belongsToThread)를 추가해 useGroupChannelThreadMessages가 내보내는 messages가 항상 이 스레드 답글만 포함하도록 했습니다. (pending echo는 parentMessageId가 아직 없을 수 있어 permissive 형태 — 다른 parent로 명시된 것만 제외).
  • react(이 PR): mirror의 parent filter를 제거하고 컬렉션 데이터를 그대로 신뢰하도록 되돌렸습니다. 지적하신 역케이스(스레드 전환 시 store의 parentId가 컬렉션 바인딩 parent보다 늦게 갱신되는 순간 정상 답글이 숨는 문제)도 이걸로 해소됩니다.

core-ts 수정은 별도 0.1.5로 배포 예정이고, 배포되면 이 PR의 @sendbird/uikit-tools^0.1.5로 올리겠습니다.

감사합니다!

Comment thread src/modules/Thread/context/ThreadProvider.tsx
Comment thread src/modules/Thread/context/useThread.ts Outdated
…tion

Follow-ups from the Thread reply-list migration review:

- Add a unified `threadMessages` state field (all replies: succeeded + pending +
  failed) and mark the split `allThreadMessages` / `localThreadMessages` as
  @deprecated (kept working for backward compatibility).
- Make the backward-compat action shims log a deprecation warning when called
  instead of being a silent no-op.
- Keep the parentMessageId scope filter in the reply mirror: the collection filters
  event/fetch results by parent, but its send/resend paths only gate on channelUrl,
  so another parent's reply (e.g. via a custom onBeforeSend*) could otherwise leak
  in. threadMessages is populated from the scoped list too.
- Clear threadMessages / allThreadMessages / localThreadMessages together on every
  reset path (parent/channel deleted, current-user ban/leave).
- Add mocking tests: state split, different-parent exclusion, reset clears all
  arrays, the deprecation warning, a pending -> succeeded reply reclassification
  with no duplicate (guards the mirror effect re-firing on an in-place status
  change), and the resendMessage non-resendable guard / send-failure catch paths.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8b06060a52

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +235 to +237
const warn = (name: string) => () => {
logger?.warning?.(`[Thread] useThread().actions.${name} is deprecated and no longer has any effect; thread state is now managed by the @sendbird/uikit-tools collection.`);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve behavior of exported thread context actions

For apps that use the exported useThreadContext() actions directly, replacing the previous state-mutating handlers with warn(...) keeps TypeScript compiling but silently drops the behavior at runtime. For example, a custom renderer or integration that calls onMessageDeleted, sendMessageFailure, or onReactionUpdated after an SDK/pubSub operation will now leave parentMessage, allThreadMessages, or localThreadMessages stale even though those actions were returned before this change. Please delegate these shims to the collection-backed state updates or treat this as a breaking API change instead of a no-op.

AGENTS.md reference: AGENTS.md:L24-L29

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 shim들은 호출 시 상태를 변경하지 않고 deprecation 경고만 남깁니다. 대응 방향은 아래와 같습니다.

  • 컬렉션으로 위임은 대부분 불가합니다. 이 dispatcher들이 다루던 상태(send 라이프사이클 sendMessageStart/Success/Failure, 리액션 onReactionUpdated, 수신 onMessageReceived 등)는 이제 @sendbird/uikit-tools 컬렉션이 내부적으로 소유하고, 이를 수동으로 주입할 공개 API가 없어 1:1 위임이 어렵습니다.
  • 제거(= 컴파일을 깨는 breaking)도 지양합니다. useThread()/useThreadContext()의 반환 shape은 public API라, 키를 제거하면 직접 참조하던 코드가 컴파일 실패합니다(backward-compat 원칙).
  • 이 action들은 외부 문서에 공개된 적 없는 내부 dispatcher입니다 (danney-chun 님도 관련 스레드 r3626818578에서 동일 취지로 코멘트하셨습니다).

그래서 shape은 유지하되(컴파일 backward-compat), silent no-op 대신 호출 시 logger.warning으로 "deprecated & 더 이상 효과 없음"을 알리는 방식으로 두었습니다 — 조용히 사라지는 게 아니라 런타임 deprecation으로 명시하고 있습니다.

감사합니다!

…react

core-ts now scopes useGroupChannelThreadMessages output to the thread on its
send/resend paths (belongsToThread) and removes deleted messages locally, so the
react-side parent filter and the deletedMessageIds delete workaround are no longer
needed. The reply mirror now reflects the collection's messages as-is.

This also resolves the review concern that the react filter could hide a legitimate
reply when the store's parent lagged the collection's bound parent — scoping is now
guaranteed at the source. Drops the now-redundant different-parent test.

(Pairs with the @sendbird/uikit-tools ^0.1.5 bump, added once 0.1.5 is published.)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8c8b5d6dd8

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/modules/Thread/context/useThread.ts Outdated
Comment on lines +30 to +32
const startingPoint = (anchorMessage && parentMessage && anchorMessage.messageId !== parentMessage.messageId)
? anchorMessage.createdAt
: parentMessage?.createdAt ?? 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reset root threads from the latest starting point

When initializeThreadFetcher() is called for a normal parent message (no separate anchor), this fallback resets the collection at parentMessage.createdAt, while the provider initially opens that same case with Number.MAX_SAFE_INTEGER in ThreadProvider.tsx. Since replies are created after the parent, a manual reinitialize/refresh jumps the collection to the oldest edge of the thread and leaves the latest replies behind hasMoreNext; use the same latest starting point for the non-anchor path and only use anchorMessage.createdAt for anchored replies.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다. 비-앵커(일반 parent) 경로가 parentMessage.createdAt(오래된 edge)에서 리셋돼 provider 최초 오픈(Number.MAX_SAFE_INTEGER, 최신 edge)과 어긋났고, 답글은 parent 이후 생성되므로 최신 답글이 hasMoreNext 뒤로 숨는 문제가 있었습니다.

비-앵커 경로를 Number.MAX_SAFE_INTEGER로 맞춰 provider와 동일하게 했고, 앵커된 답글(특정 메시지로 진입한 경우)만 anchorMessage.createdAt을 쓰도록 유지했습니다. root(MAX) / anchored(anchor createdAt) 두 케이스에 대한 테스트도 추가했습니다.

감사합니다!

… the provider

initializeThreadFetcher() reset a normal parent thread (no separate anchor) at
parentMessage.createdAt, i.e. the oldest edge — replies are created after the parent,
so this left the latest replies behind hasMoreNext. The provider opens that same case
at Number.MAX_SAFE_INTEGER (latest edge), so a manual reinitialize disagreed with the
initial open.

Use MAX_SAFE_INTEGER for the non-anchor path so both paths match; only an anchored
reply (entered from a specific message) still resets at the anchor's createdAt.

Adds tests asserting the reset starting point for both the root (MAX) and anchored
(anchor createdAt) cases.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a1b15e4964

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +223 to +226
updateState({
currentChannel: null,
channelState: ChannelStateTypes.NIL,
threadListState: ThreadListStateTypes.NIL,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Clear parent state when the channel is deleted

When the collection reports onChannelDeleted, this reset leaves parentMessage and parentMessageState untouched, unlike the current-user-banned path below. In a deleted-channel scenario the thread can still render the stale parent message and the composer is not disabled by ThreadMessageInput because its guard only checks parentMessage === null in addition to online/mute/frozen state, so attempts to send are presented to the user even though currentChannel has been cleared and the send actions silently no-op. Clear the parent state (or close the thread) as part of this callback.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다. onChannelDeleted 리셋에 parentMessage: null / parentMessageState: NIL을 추가했습니다(current-user-banned 경로와 동일하게 정리). 이제 채널이 삭제되면 parentMessage가 null이 되어 ThreadMessageInput이 composer를 비활성화하므로, currentChannel이 없어 no-op되는 전송을 사용자에게 보여주지 않습니다. onChannelDeleted가 parent/channel/reply 배열을 모두 정리하는지 확인하는 회귀 테스트도 추가했습니다.

감사합니다!

Comment on lines 86 to 88
const MemorizedThreadList = useMemorizedThreadList({
threadListState,
threadListState: parentMessageState === ParentMessageStateTypes.INITIALIZED ? threadListState : ThreadListStateTypes.NIL,
renderThreadListPlaceHolder,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Hide stale reply counts while the parent is unavailable

This gates only the thread list on parentMessageState, but replyCount is still computed from the persisted allThreadMessages above. When the same <ThreadProvider> switches threads, getParentMessageStart sets the parent to loading/null while the old replies remain in the store until the new collection mirrors messages, so the UI can show the previous thread's reply count above the placeholder even though the list itself is hidden. Gate the count on the same initialized parent state or clear the reply arrays during the parent reset.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다. replyCount를 thread list와 동일하게 parentMessageState === INITIALIZED 기준으로 게이트했습니다(parentMessageState === INITIALIZED ? allThreadMessages.length : 0). 스레드 전환 중에는 parent가 loading/null인 동안 이전 스레드 replies가 store에 남아 있어도 stale 카운트가 노출되지 않고, 리스트와 카운트가 같은 기준으로 함께 숨겨집니다. 카운트가 parent 초기화 후에만 표시되는지 / 초기화 전에는 stale replies가 남아 있어도 표시되지 않는지 확인하는 회귀 테스트도 추가했습니다.

감사합니다!

…oser disables

onChannelDeleted cleared the channel and reply arrays but left parentMessage /
parentMessageState intact. ThreadMessageInput only disables on parentMessage === null
(plus online/mute/frozen), so with a stale parent the composer stayed enabled after the
channel was gone and sends silently no-op'd (currentChannel was cleared). Clear the
parent state too, mirroring the current-user-banned reset.

Adds a regression test: onChannelDeleted clears parentMessage/parentMessageState, the
channel, and all reply arrays.
…a stale count

ThreadUI derived replyCount from allThreadMessages but only the thread list was gated on
parentMessageState. On a thread switch the previous thread's replies linger in the store
until the new collection mirrors, so the header could show the old thread's reply count
above the (already hidden) list. Gate the count on the same initialized-parent state as
the list.

Adds tests: the count shows once the parent is initialized, and is hidden while the
parent is not yet initialized even though stale replies remain in the store.

@danney-chun danney-chun left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생 하셨습니다.~
PR Label에 배포될 버전을 표시해 주시면 좋을 것 같습니다.

@sf-tyler-jeong

Copy link
Copy Markdown
Contributor Author

고생 하셨습니다.~ PR Label에 배포될 버전을 표시해 주시면 좋을 것 같습니다.

넵~ 감사합니다!

0.1.5 ships the published CLNP-8740 thread Collection fixes: local send/resend scoped to the parent, and local delete removal when onMessagesDeleted does not fire for the thread. Bump @sendbird/uikit-tools and @sendbird/react-uikit-message-template-view from ^0.1.4 to ^0.1.5 and update the lockfile to the published versions, replacing the interim local build.
@sf-tyler-jeong
sf-tyler-jeong merged commit 0b5fe33 into main Jul 23, 2026
8 checks passed
@sf-tyler-jeong
sf-tyler-jeong deleted the fix/CLNP-8740 branch July 23, 2026 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants