refactor: remove MobX

[jvorcak]

Removes observer() from components that don't access MobX observables directly:

• error-display.tsx, error401-page.tsx, kowl-json-view.tsx
• operation.tsx, require-auth.tsx
• message-headers.tsx, message-meta-data.tsx, payload-component.tsx

[malinskibeniamin]

@claude review mobx updates

[claude]

Claude finished @malinskibeniamin's task in 4m 54s —— View job

---

MobX Refactor Review

• Read CLAUDE.md for repo guidelines
• Analyze PR structure and scope
• Review key changed files for correctness
• Check for potential issues with MobX removal
• Post review findings

---

Summary

This is a large, multi-session refactor removing observer() and useLocalObservable from 100 components. The direction is correct and the overall approach is sound. A few concrete issues and architectural concerns are worth noting before merging.

---

🔴 Critical Bug — login.tsx stale Zustand reads

File: frontend/src/components/misc/login.tsx:220-237

LoginPage reads uiState.loginError directly in JSX:

<Modal isOpen={uiState.loginError !== null} ...>
  <Text>{uiState.loginError}</Text>
  <Button onClick={() => { uiState.loginError = null; }}>OK</Button>

uiState is a Proxy that reads from useUIStateStore.getState() — but accessing it this way in render does not subscribe the component to the Zustand store. When another part of the app sets uiState.loginError = "some error", LoginPage will not re-render, and the error modal will never open.

Fix: Replace direct reads with the Zustand hook:

const loginError = useUIStateStore((s) => s.loginError);
const setLoginError = useUIStateStore((s) => s.setLoginError);
// Use loginError / setLoginError(null) in JSX

Fix this →

---

🟡 Broken Feature — topic-input.tsx non-reactive mutation

File: frontend/src/components/pages/connect/dynamic-ui/forms/topic-input.tsx:59

const cycleError = showErrors
  ? () => { property.currentErrorIndex += 1; }
  : undefined;

property.currentErrorIndex was previously a MobX observable field. Now that it's mutated directly on a plain object and the component is not an observer, calling cycleError won't trigger any re-render. The error-cycling UI will appear broken (clicks do nothing visually).

Fix: Track currentErrorIndex in local React state:

const [errorIndex, setErrorIndex] = useState(0);
const errorToShow = showErrors ? errors[errorIndex % errors.length] : undefined;
const cycleError = showErrors ? () => setErrorIndex(i => i + 1) : undefined;

---

🟡 Architecture Concern — active-reassignments.tsx class component lost @observer

File: frontend/src/components/pages/reassign-partitions/components/active-reassignments.tsx:86-95

The ActiveReassignments class component's render() method reads api.brokerConfigs (still a MobX observable map on the unchanged backend-api.ts):

const leaderThrottle = [...api.brokerConfigs.values()]...

The @observer decorator has been removed from this class, so changes to api.brokerConfigs will no longer trigger re-renders. Throttle settings may appear stale until a full page reload.

---

🟡 Stale Closure — list.tsx useEffect intentionally skips onChange

File: frontend/src/components/pages/connect/dynamic-ui/list.tsx:97-100

// biome-ignore lint/correctness/useExhaustiveDependencies: props.onChange is intentionally excluded
useEffect(() => {
  props.onChange(data.map((x) => x.id).join(','));
}, [data]);

The biome-ignore suppresses a real issue. If the parent passes an inline () => ... as onChange, the effect will always call a stale version. A useRef pattern avoids the suppression while keeping correct semantics:

const onChangeRef = useRef(props.onChange);
useEffect(() => { onChangeRef.current = props.onChange; });
useEffect(() => {
  onChangeRef.current(data.map((x) => x.id).join(','));
}, [data]);

---

🟢 Good Patterns

• error-modal.tsx: The subscriber-set + useReducer replacement for MobX is well-designed. Replaces the global observable array with an explicit notification channel — correct and idiomatic.
• login.tsx auth refactor: Extracting NoneAuthComponent, BasicAuthComponent, OidcAuthComponent as separate FCs is a net improvement in readability.
• user-preferences.tsx: The dual-write approach (update both React state and uiSettings on change) is a clean way to remove MobX without a full Zustand migration.
• CommaSeparatedStringList: Clean class-to-FC conversion with arrayMoveMutable used immutably via [...prev] copy.

---

📄 Migration Notes Files in Source Tree

MOBX_MIGRATION_STATUS.md and MOBX_MIGRATION_NEXT.md have been committed to frontend/. These are valuable during the migration, but once merged they'll become stale immediately and may confuse future contributors. Consider removing them before merge or moving them to a .claude/ or similar internal docs folder.

---

Broad Concern: api.* reads without observer()

Many migrated components still access api.* (e.g., api.topicConfig, api.brokerConfigs) which is still MobX-backed (backend-api.ts still imports from mobx). With observer() removed from all components (confirmed: no observer() usage remains in src/components/), these reads are no longer reactive. Whether this is a problem depends on whether the data is always loaded before the component renders (via route loaders / React Query). This should be explicitly validated for data-dependent pages to avoid showing stale DefaultSkeleton states.

[malinskibeniamin]

Let's double check if that still works when in embedded mode with cloud UI, sidebar items need to refresh

[malinskibeniamin]

I think this deserves its own utility function

[malinskibeniamin]

Not set or Any?

[malinskibeniamin]

I guess functional components can be used later on?

[malinskibeniamin]

Do we still need to keep the observable if we removed the observer pattern?

[jvorcak]

it's just a variable name, I see it's not optimal, but I think this can be addressed later on, I want to make as few changes as possible.

[malinskibeniamin]

I think it should be its own component or utility method

[malinskibeniamin]

Let's double check embedded mode functionality with cloud UI route syncing

[malinskibeniamin]

@claude review extensively the state management logic. We need to make sure business logic remain intact. Look into any potential side effects and double check all observable patterns are properly substituted to ensure no regressions. Also investigate Kafka connect/reassign partitions features to endure they continue to be operational.

[claude]

Claude finished @malinskibeniamin's task in 7m 8s —— View job

---

Deep State Management Review

• Read CLAUDE.md for repo guidelines
• Review Kafka Connect state management
• Review Reassign Partitions state management
• Audit observable patterns / Zustand substitutions
• Check for business logic regressions
• Post comprehensive findings

---

State Management Review

This is a deep audit of the reactivity migration focusing on Kafka Connect and Reassign Partitions. Several critical and high-priority issues were found.

---

🔴 Critical — ConfigPage Form/JSON toggle and "Show Advanced Options" are broken

File: frontend/src/components/pages/connect/dynamic-ui/components.tsx:64-82

ConfigPage reads connectorStore.viewMode and connectorStore.showAdvancedOptions directly from a plain ConnectorPropertiesStore class instance. When the user clicks "JSON" or toggles advanced options, the mutations go to the plain object but nothing triggers a React re-render:

<RadioGroup
  onChange={(x) => { connectorStore.viewMode = x; }}  // ← plain object mutation
  value={connectorStore.viewMode}                      // ← no subscription
/>

{connectorStore.viewMode === 'form' ? <FormView /> : <JsonView />}  // ← never switches

ConnectorPropertiesStore does have a subscribe()/notifyChange() mechanism, but it is only used internally (for JSON sync and validation loops). No React component subscribes to it for re-renders.

Impact: The Form/JSON tab switch in both the Create Connector wizard and the Edit Connector page is completely non-functional. Same for the "Show advanced options" toggle.

Fix: ConfigPage needs to subscribe to the store and force re-renders, or lift viewMode/showAdvancedOptions into React state:

const [viewMode, setViewMode] = useState(connectorStore.viewMode);
const [showAdvanced, setShowAdvanced] = useState(connectorStore.showAdvancedOptions);
// Update both React state and connectorStore on change

Fix this →

---

🔴 Critical — PropertyComponent controlled inputs are frozen

File: frontend/src/components/pages/connect/dynamic-ui/property-component.tsx

Every property input uses a controlled pattern reading directly from the plain Property object:

const v = p.value;
// ...
<Input
  value={String(v)}
  onChange={(e) => { p.value = e.target.value; }}  // ← plain object mutation, no re-render
/>

In React, a controlled input whose value prop doesn't change (because no state update triggers a re-render) will revert any character the user types. All string, list, radio, and number inputs in the connector property form are affected. Password (SecretInput) and CommaSeparatedStringList are not affected because they use their own useState internally.

Impact: Users cannot type in any text/numeric property field in the Kafka Connect configuration form.

Root cause: Property objects are plain objects; mutations to p.value do not propagate to React. The ConnectorPropertiesStore.notifyChange() subscription is only used internally for JSON sync/validation and does not trigger component re-renders.

Fix this →

---

🔴 Critical — ConnectorPropertiesStore.initPending never causes re-render

File: frontend/src/components/pages/connect/dynamic-ui/components.tsx:40-41

if (connectorStore.initPending) {   // ← reads plain object property
  return <Skeleton height={4} mt={5} noOfLines={20} />;
}

initPending is set to false at the end of ConnectorPropertiesStore.initConfig() (an async operation). When it transitions from true → false, there is no call to notifyChange() and no Zustand update, so the ConfigPage does not re-render. The skeleton is displayed permanently.

The same pattern applies to ConnectClusterStore.isInitialized in KafkaConnectorMain (connector-details.tsx:109):

if (!connectClusterStore.isInitialized) {
  return <Skeleton height={4} mt={5} noOfLines={20} />;
}

This relies on a fragile race condition — an earlier Zustand store update from api.refreshClusterAdditionalInfo() triggers forceUpdate() before isInitialized = true is set, and then isInitialized may or may not be true by the time React flushes the re-render. This is non-deterministic.

Fix this →

---

🟡 High — currentErrorIndex cycling is non-reactive in two places

Files:

• frontend/src/components/pages/connect/dynamic-ui/forms/error-wrapper.tsx:21
• frontend/src/components/pages/connect/dynamic-ui/forms/topic-input.tsx:59

Both compute errorToShow from property.currentErrorIndex % errors.length and expose a cycleError callback that does property.currentErrorIndex += 1. Since neither component has state for this index, clicking on the error label will increment the plain object property but will not re-render the component — the displayed error will never cycle.

const cycleError = showErrors
  ? () => { property.currentErrorIndex += 1; }  // ← no re-render
  : undefined;

Fix: Move currentErrorIndex to local useState in each component (or share via a wrapper that re-renders):

const [errorIndex, setErrorIndex] = useState(property.currentErrorIndex);
const errorToShow = showErrors ? errors[errorIndex % errors.length] : undefined;
const cycleError = showErrors ? () => setErrorIndex(i => i + 1) : undefined;

---

🟡 High — ReassignmentTracker.trackingReassignments updates may miss renders

File: frontend/src/components/pages/reassign-partitions/components/active-reassignments.tsx:113

const currentReassignments = reassignmentTracker.trackingReassignments ?? [];

reassignmentTracker runs a 3-second timer that mutates this.trackingReassignments in-place (push(), filter(), assignment). Since ActiveReassignments is no longer an @observer, these mutations don't directly trigger re-renders.

How it survives: refreshReassignments() calls api.refreshPartitionReassignments(true) and api.refreshPartitions(topics, true), which update the Zustand store → triggers PageComponent.forceUpdate() → cascades to ActiveReassignments. So the table does update, but only when the API calls complete, not when the tracker's in-memory array is updated. This means progress, ETA, and speed estimates (computed entirely in-memory in updateReassignmentState()) may lag by one render cycle.

Minor risk: If an API call is cached/deduped and doesn't produce a Zustand update, the reassignment table could stay stale for the full 3-second interval. Not a regression in correctness, but worth noting.

---

🟡 Medium — PropertyGroupComponent reads g.filteredProperties which depends on showAdvancedOptions

File: frontend/src/components/pages/connect/dynamic-ui/property-group.tsx:31

const filteredProperties = g.filteredProperties;

filteredProperties is a getter on PropertyGroup:

get filteredProperties(): Property[] {
  if (self.showAdvancedOptions) { return this.properties; }
  return this.properties.filter(p => p.entry.definition.importance === PropertyImportance.High);
}

Since self.showAdvancedOptions (from ConnectorPropertiesStore) is a plain object read and the component has no subscription, even if the Form/JSON toggle issue is fixed, the filtered property list will not update when showAdvancedOptions changes. The number of visible properties is frozen at the initial value.

This composes with the ConfigPage issue above but also affects ConnectorStepComponent:30 which reads groups.sum(x => x.filteredProperties.length).

---

🟢 Reassign Partitions — core functionality intact

ReassignPartitions is a PageComponent subclass, so its componentDidMount subscribes to useApiStore.subscribe(update) → forceUpdate(). All reads of api.clusterInfo, api.topics, api.topicPartitions, and api.partitionReassignments in render() are refreshed whenever the Zustand store updates. The throttle dialog and bandwidth slider both use proper useState. Broker config throttle reads in ActiveReassignments go through the same forceUpdate() cascade.

The reassignment wizard (steps 1–3) uses this.state via setState() for selection tracking — correct React class component pattern. The ReassignmentTracker timer and progress calculation logic are preserved faithfully.

---

🟢 Error Modal migration is solid

frontend/src/components/misc/error-modal.tsx — The subscriber-set + useReducer pattern replacing the MobX observable array is correct and idiomatic. External callers use showErrorModal() which dispatches to the reducer, and the component renders from the queue. No regressions.

---

📄 Summary

Area	Status
Kafka Connect — Form/JSON toggle	🔴 Broken
Kafka Connect — Property inputs	🔴 Broken (controlled inputs frozen)
Kafka Connect — Loading skeleton	🔴 Non-deterministic/stuck
Kafka Connect — Error cycling	🟡 Non-functional click
Reassign Partitions — Core flow	🟢 Works via PageComponent subscription
Reassign Partitions — Tracker timing	🟡 Slightly indirect but functional
Error Modal	🟢 Correct
Router sync / sidebar	🟢 Correct (proper hooks)

The Kafka Connect configuration UI has several interconnected reactivity gaps stemming from ConnectorPropertiesStore being a plain class whose mutations no React component subscribes to. Before merging, the Form/JSON switch, Show Advanced Options, and all controlled property inputs should be validated end-to-end in a browser. The fastest fix is to lift viewMode, showAdvancedOptions, and initPending transitions into React state (or add a useEffect that subscribes to connectorStore.subscribe() and calls a local state setter to trigger re-renders).

[malinskibeniamin]

big moment if we can finally get there. thank you

[malinskibeniamin]

also regenerate yarn.lock

[jvorcak]

yarn is not installed in this environment — can you regenerate yarn.lock locally with yarn install after merging bun.lock?

[malinskibeniamin]

You can do "bun install --yarn"

[malinskibeniamin]

would this be sufficient? this would mean sidebar items get updated twice, I just want to make sure no race conditions appear.

[jvorcak]

Good question — no race condition here. The two calls are intentional: the first fires immediately so the sidebar is populated on startup (in case endpointCompatibility is already set); the second fires when it changes (e.g., after the initial API fetch populates it from null). Since updateSidebarItems() reads the current store snapshot and calls setSidebarItems() synchronously, calling it twice is harmless — the second just overwrites with the same or more accurate result.

[malinskibeniamin]

why not keep using MAX_PAGE_SIZE?

[malinskibeniamin]

Do we need useState hook if we are never changing the state for the tags? I think they will always remain the same based on this code?

[malinskibeniamin]

should it be info instead? resizing pipeline should not be a warning, it's just a message

[malinskibeniamin]

nevermind, that was the functionality before, fine with either

[malinskibeniamin]

Does this logic still exist anywhere? Do we still need it?

[jvorcak]

Yes, it's still needed — api.topicPartitions.get(topic.topicName) is read in tab-partitions.tsx. Removed the stale comment about 'statistics bar' and updated it to reflect the actual usage.

[malinskibeniamin]

not sure if still relevant given mobx is fully cleaned up

[jvorcak]

You're right — removed. PageComponent now re-renders via Zustand store subscriptions (useApiStore.subscribe(forceUpdate)), so the MobX-style observable touches are no longer needed.

[malinskibeniamin]

nit: having anonymous arrow function called makes it hard to read but it's fine for now

[secpanda]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[malinskibeniamin]

In the future we could replace it by using tanstack query polling mode