unbridled-41 opened a new pull request, #4590:
URL: https://github.com/apache/rocketmq-dashboard/pull/4590
Fixes #4589.
## Problem / Evidence
The instance discovery effect on the broker topology page writes the first
Apache-capable instance into the selection on every run:
```tsx
useEffect(() => {
let active = true;
void listInstances()
.then((nextInstances) => {
…
setSelectedInstanceId(apacheInstances[0]?.name); // :236,
unconditional
})
…
}, [clearData, message, t]); // :246
```
`t` is recreated whenever the display language changes
(`web/src/i18n/LangContext.tsx`), while `clearData` and `message` are stable —
so the top-bar language toggle re-runs the effect and overwrites the instance
the user selected. The data below then reloads for that other instance. The
twin page already guards the identical write
(`web/src/pages/studio/Producer.tsx:104-108`), which is the intended contract.
Reproduced by the regression test added here: it selects `instance-2`,
switches the language through the real `LangProvider.setLang`, and asserts both
the loaded topology and the instance the page queries.
## Root cause / Fix
An effect that both discovers instances and seeds the default selection,
keyed on a dependency that changes for an unrelated reason (rendering text in
another language). The minimal fix is the guard the sibling page uses — the
effect may re-run, but it only replaces a selection that is no longer valid:
```diff
const apacheInstances = nextInstances.filter(supportsApacheRuntime);
setInstances(apacheInstances);
- setSelectedInstanceId(apacheInstances[0]?.name);
+ setSelectedInstanceId((current) =>
+ apacheInstances.some((instance) => instance.name === current)
+ ? current
+ : apacheInstances[0]?.name,
+ );
```
A first visit still selects the first Apache instance, and an instance that
disappeared from the list still falls back — only a re-run for an unrelated
reason keeps the user's choice.
## Priority & scoring
- PRIORITY **70** = impact 26 + blast radius 9 + reproducibility 20 +
maintenance value 15
- impact 26 — the page silently switches to another instance's
broker/nameserver/proxy topology; a refresh or a routine language toggle is
enough to make the user read the wrong instance's data.
- blast radius 9 — one page, but it is the page used to inspect broker
topology in multi-instance deployments; the topology is also the input to the
config actions on that page.
- reproducibility 20 — deterministic; the regression test drives the real
language switch and fails on `master`.
- maintenance value 15 — the same effect exists on the producer page with
the guard in place; aligning them removes the divergence.
- FIX_CONFIDENCE **95** — the sibling implementation defines the expected
behaviour exactly, the change is three lines, and the page's other 15 tests
(including "clears topology from the previous instance when the next instance
fails to load") still pass.
## Tests
Environment: Node 24.20.0, `web/` at the PR head.
Red — base source (`d50ffecc`) with the new test:
```
$ git checkout origin/master -- web/src/pages/studio/BrokerCluster.tsx
$ npx vitest run
src/pages/studio/__tests__/BrokerClusterInstanceScope.test.tsx
FAIL src/pages/studio/__tests__/BrokerClusterInstanceScope.test.tsx >
BrokerCluster instance scope > keeps the instance the user selected when the
display language changes
AssertionError: expected last "vi.fn()" call to have been called with [
'instance-2' ]
- Expected
+ Received
[
- "instance-2",
+ "instance-1",
]
❯ src/pages/studio/__tests__/BrokerClusterInstanceScope.test.tsx:160:46
Test Files 1 failed (1)
Tests 1 failed (1)
```
Green — with the fix, including the page's existing suite:
```
$ npx vitest run
src/pages/studio/__tests__/BrokerClusterInstanceScope.test.tsx \
src/pages/studio/__tests__/BrokerCluster.test.tsx
Test Files 2 passed (2)
Tests 16 passed (16)
```
Full suite:
```
$ npx vitest run --maxWorkers=4
Test Files 1 failed | 122 passed (123)
Tests 1 failed | 1034 passed (1035)
```
`1035 = 1034 (pristine origin/master) + 1 new test`. The single failure is
`ConsumerPage.test.tsx > keeps the latest client stack when an older request
resolves last` — a file this PR does not touch and one of the load-fragile
cases on this repository (the same file also fails intermittently on pristine
`master` under a full parallel run). Isolating it:
```
$ npx vitest run src/pages/instance/__tests__/ConsumerPage.test.tsx
Test Files 1 passed (1)
Tests 32 passed (32)
```
Static checks:
```
$ npx tsc -b # clean, exit 0
$ npx eslint src/pages/studio/BrokerCluster.tsx
src/pages/studio/__tests__/BrokerClusterInstanceScope.test.tsx
# no output: 0 errors, 0 warnings
$ npm run build # ✓ built in 12.48s
```
Diff: `web/src/pages/studio/BrokerCluster.tsx` +5/−1,
`web/src/pages/studio/__tests__/BrokerClusterInstanceScope.test.tsx` +165/−0
(`git show --numstat`).
## Risk
- The effect still re-runs on a language change (it re-reads the instance
list and re-renders its labels); only the destructive part — replacing a
still-valid selection — is removed. Making the effect mount-only would
additionally require routing the failure message through a ref; that is
deliberately left out to keep this change to the reported defect.
- Fallback behaviour is unchanged for the cases that need it: no instances →
`undefined` selection, selected instance removed from the list → first Apache
instance.
- No API, contract or translation-key changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
