Skip to content

Fix other places where useApiStoreHook might be missing#2355

Merged
jvorcak merged 2 commits intomasterfrom
fix/use-api-store-hook
Apr 1, 2026
Merged

Fix other places where useApiStoreHook might be missing#2355
jvorcak merged 2 commits intomasterfrom
fix/use-api-store-hook

Conversation

@jvorcak
Copy link
Copy Markdown
Collaborator

@jvorcak jvorcak commented Mar 31, 2026

Our goal is to move as much data fetching inline to the components using hooks. But until then, we can use zustand and there were a couple of places missing useApiStoreHook after mobx migration. e.g.
#2354

@jvorcak
Copy link
Copy Markdown
Collaborator Author

jvorcak commented Mar 31, 2026

@claude review this

@claude
Copy link
Copy Markdown

claude Bot commented Mar 31, 2026

Claude finished @jvorcak's task in 1m 28s —— View job


Code Review

  • Gather context on changed files
  • Read key changed files
  • Review changes for correctness and consistency
  • Post review feedback

The PR continues the MobX → Zustand migration by replacing direct api.* property accesses with useApiStoreHook(...) subscriptions. Overall the approach is correct and consistent. One bug found, a few minor observations:


Bug: api.clusterHealth still accessed directly in tab-partitions.tsx

tab-partitions.tsx was partially migrated — topicPartitions was converted, but api.clusterHealth on lines 47 and 50 was left as a direct access. This means the leaderless/under-replicated partition badges won't re-render when cluster health data changes.

// tab-partitions.tsx:47-51 — still using api directly
const leaderLessPartitions = (api.clusterHealth?.leaderlessPartitions ?? []).find(...)
const underReplicatedPartitions = (api.clusterHealth?.underReplicatedPartitions ?? []).find(...)

Should be:

const clusterHealth = useApiStoreHook((s) => s.clusterHealth);
const leaderLessPartitions = (clusterHealth?.leaderlessPartitions ?? []).find(...)
const underReplicatedPartitions = (clusterHealth?.underReplicatedPartitions ?? []).find(...)

Fix this →


Minor observations

group-details.tsx — The consumerGroupAcl selector chains on group?.groupId where group itself comes from a prior hook call. This is technically fine (both hooks subscribe independently and will trigger re-renders correctly), but it means consumerGroupAcls.get('') is called when the group isn't loaded yet. Not harmful since it'll just return undefined, but worth noting.

topic-details.tsx — Removing useSyncExternalStore(useApiStore.subscribe, useApiStore.getState) in favour of targeted selectors is a clear improvement in precision.

delete-records-modal.tsx — The original code used api.topicPartitionErrors?.get(topicName) with optional chaining (?.get), which was defensive but unnecessary since the Map is always initialized. The new code correctly drops the ?.


Overall: clean, consistent migration. Fix the tab-partitions.tsx clusterHealth miss and this looks good to merge.

@jvorcak jvorcak marked this pull request as ready for review March 31, 2026 11:48
@jvorcak jvorcak requested a review from a team March 31, 2026 11:48
@jvorcak jvorcak merged commit 530f3a1 into master Apr 1, 2026
17 checks passed
@jvorcak jvorcak deleted the fix/use-api-store-hook branch April 1, 2026 10:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants