moonming opened a new issue, #3417:
URL: https://github.com/apache/apisix-dashboard/issues/3417
This is the tracking issue for a full review of the rewritten dashboard
(`master @ 9979b31`), done with static analysis cross-checked against a live
APISIX instance. Every item below was verified by reading the code at the cited
location and/or reproducing at runtime — items are grouped by theme, each with
its pointer. Dedicated issues/PRs are linked where they exist; unlinked items
are up for grabs (several are `good first issue` material).
## Already tracked separately
- [ ] #3414 — **CRITICAL** no-op Edit→Save deletes inline upstream health
checks (silent data loss; runtime-reproduced)
- [ ] #3415 — **CRITICAL** 401 fabricated into a successful `{data:{}}`
response (false success toasts, crashes)
- [ ] #3416 — secrets/private keys/tokens plaintext in view mode
- [ ] #3412 — Plugin Metadata page false "Key not found" toasts (root cause
identified in the issue: skip list says `'500'`, Admin API returns `404`)
- [ ] #3296 — `zOneOf` inverted xor (dead code today; fix or remove)
- [ ] #3205 — no search on list pages. Verified scope: **all 12** list pages
set `search={false}`; worse, 5 pages (consumers, services, plugin_configs,
consumer_groups, credentials) declare `sorter: true` with no handler — the sort
arrows render but do nothing. The plumbing already exists end-to-end
(`pageSearchSchema` reserves `name`/`label`; the request interceptor already
serializes Admin API `filter`) — only the input box is missing.
- [ ] #3322 — dark mode. Cost note: nothing in `src/` is theme-aware today;
the expensive part is the antd+Mantine dual-theming — nearly free after
UI-library consolidation (below).
## Error handling / resilience
- [ ] Network-level failure (Admin API unreachable/timeout) of any mutation
gives **zero feedback**: the interceptor only handles `err.response` errors
(`src/config/req.ts`), the shared `QueryClient` has no `MutationCache.onError`
(`src/config/global.ts`), and 25 of 26 `useMutation` sites have no local
`onError`. Fix in one place: `new QueryClient({ mutationCache: new
MutationCache({ onError }) })` toasting on `!err.response`.
- [ ] No `defaultErrorComponent` on the router: 11 of 12 resources render
loader/render failures as the dev-styled red stack dump with no retry (only
stream_routes registers a custom one — and that one dereferences
`err.response?.data.error_msg` without guarding `data`, the exact #3370 shape).
- [ ] `routes/detail.$id.tsx` has the app's only local mutation `onError` —
it double-toasts on HTTP errors (interceptor already toasted) with hardcoded
English strings.
- [ ] Plugin metadata hook classifies *any* failed GET as "not configured" →
a real 500/network failure on a configured plugin offers the user an empty `{}`
to save over their existing metadata (detailed in #3412 comment).
- [ ] `PluginEditorDrawer` fires `onSave` (async `mutateAsync` on the
metadata page), then closes and resets **without awaiting** — a failed save
loses the user's edits.
- [ ] `secrets/detail.$manager.$id.tsx` is the only non-suspense detail
page: on fetch failure it renders a blank editable form (Save would PUT an
empty object over the real secret) instead of an error state.
- [ ] Route `vars` invalid JSON: with a zod resolver attached, RHF ignores
the Monaco field-level `validate` rule, so `JSON.parse` throws inside
`mutationFn` → on `routes/add` the Add click does nothing, silently.
- [ ] `?page=abc` becomes `NaN` and is sent to the Admin API; duplicated
`?page=` params throw a ZodError into the raw boundary
(`src/types/schema/pageSearch.ts`).
- [ ] Settings modal admin-key field fires `queryClient.invalidateQueries()`
+ `refetchQueries()` **per keystroke**
(`src/components/page/SettingsModal.tsx`) — captured an 18-request burst of
identical list GETs; typing a 32-char key ≈ 32 full refetch storms. Apply on
blur/debounce.
## Data-integrity (form ↔ API round-trip)
- [ ] `rmDoubleUnderscoreKeys` recurses into `null` (`typeof null ===
'object'`) and throws — runs *before* the null-cleaner in `pipeProduce`, so a
plugin config containing `null` crashes the submit (`src/utils/producer.ts`).
- [ ] Empty-value stripping inside surviving plugin configs:
`produceRestoreEmptyPlugins` only restores whole-plugin entries; nested
meaningful empties (`{"headers": {}}`, `[]`, `""`) are still silently removed
(sibling of #3269/#3277). Inline route/service `upstream.discovery_args: {}`
also still dropped (the #3376 merge-back fix covered the upstreams page only).
- [ ] Inline upstream `name`/`desc`/`labels` are unconditionally `delete`d
on submit (`FormPartUpstream/util.ts`) while the form renders inputs for them —
user input silently discarded; API-created values lost on any edit-save.
- [ ] Route `script`/`script_id` have no form control; with
`shouldUnregister: true` they're dropped from the PUT body — a scripted route
loses its script on any dashboard edit.
- [ ] `checks.active.http_request_headers` is rendered with the Labels
widget (object) but typed `array<string>` — existing values display empty, new
input fails zod: field unusable in both directions (`FormSectionChecks.tsx`).
- [ ] `ssls.client.skip_mtls_uri_regex` rendered as a boolean Switch but
typed `array<string>` — same both-directions breakage (`FormPartSSL/index.tsx`).
- [ ] Labels widget splits on every `:` — a label value containing a colon
errors on any subsequent edit of the tag set (`Labels.tsx`; split on first
colon only).
- [ ] Object-form nodes conversion corrupts IPv6 hosts and invents `port: 1`
/ `priority: 0` (`FormItemNodes.tsx` `key.split(':')`).
- [ ] Stream routes: create path bypasses `pipeProduce` (only resource whose
create/edit run different cleaning pipelines), and the reused plugins section
offers a `plugin_config_id` input the resource schema doesn't have — typed
values silently vanish.
- [ ] Consumer delete redirects to `/consumer_groups` instead of
`/consumers` (`consumers/detail.$username/index.tsx` — copy-paste).
## UX / IA / i18n / a11y
- [ ] Dirty-form navigation is fully unguarded (no
`useBlocker`/`beforeunload` anywhere): sidebar click, browser Back, or tab
close silently discards edits; the 14 add pages have no Cancel at all, so
navigation is their *only* exit. Conversely `useEditCancelGuard` always
interrogates even with zero changes (its own comment documents why `isDirty` is
untrustworthy under the `disabled`-toggling architecture — worth fixing at the
root: mount the form only in edit mode).
- [ ] Language choice is never persisted or detected (`lng: 'en'` hardcoded;
reload resets a zh/tr user to English — while the admin key *is* persisted);
antd `ConfigProvider` is hardwired to `enUS` regardless of app language, so
list pages stay English after switching; delete-confirm builds sentences by
concatenation, producing garbled output in de/tr/es (trailing "¿?");
translation-progress indicator counts keys not values (all locales report 100%
while tr has 98/232 English values) so it never displays.
- [ ] Accessibility: exactly **one** `aria-label` in all of `src/` —
language/settings/burger buttons and plugin-card action buttons are nameless to
screen readers; view-mode Monaco text is ~2.1:1 contrast (gray-5 on gray-0,
`global.css`) on the primary read path; both drawers set
`closeOnEscape={false}` (for the select drawer there's no state to protect);
plugin cards are wrapped in `Combobox`/`role="option"` semantics with nested
interactive buttons and no keyboard wiring. Zero a11y assertions in e2e.
- [ ] IA: flat 12-item nav with no grouping/icons; no breadcrumbs
(4-level-deep credential pages orient by browser Back); one static
`document.title` for every page (router `HeadContent` is already mounted — one
`head` option per route away).
- [ ] Form TOC never refreshes when sections appear dynamically:
`refreshTOC` builds a debounced function and discards it without calling
(`FormSection/index.tsx:149-152`).
- [ ] Timestamps rendered as raw UTC ISO strings in list columns (`new
Date(...).toISOString()`), inconsistent with detail views.
## Architecture / hygiene (the recurring-bug factories)
- [ ] **Dual UI library**: antd + pro-components exist only for `ProTable`
on the 12 list pages (configured down to `search={false} options={false}`) +
one `EditableProTable`; everything else is Mantine. Two token systems, two
focus rings, two error styles on one screen; both frameworks load on the
default landing route. Replace with a Mantine-native table.
- [ ] **MobX toolchain held hostage by one component**:
`mobx-react-observer`'s SWC plugin wraps every component app-wide to serve a
single `useLocalObservable` search filter in `PluginCardList.tsx` (replaceable
by `useState`+`useMemo`); `mobx-persist-store` has **zero** imports — dead
dependency.
- [ ] **Hand-written zod transcription of Admin API constraints** (~1,077
lines) is the root of the recurring "form rejects what the API accepts" class
(#3146, #3147, #3362, #3376, #3395…). Cheapest structural guard: a CI contract
test asserting the zod layer is *looser or equal* to the gateway's own schema;
longer term, the JSON-Schema-driven form work (#3347/#3311/#2986).
- [ ] **Submit pipeline strips then restores**: `pipeProduce` deep-cleans
all empty values then patches back known exceptions one incident at a time —
every future meaningful-empty field is pre-broken. Invert to dirty-fields-based
submission.
- [ ] `apis/` imports types from `components/` (inverted layering, e.g.
`apis/ssls.ts`); one detail page re-declares an exported query-options helper.
- [ ] The repo's only unit test (`upstreams.test.ts`, guarding the #3376
fix) **never runs in CI** — no `test` script, no workflow invokes vitest.
## Test-suite gaps (would-be regression escapes)
- [ ] No round-trip invariant test (create → read → no-op edit-save →
deep-diff) — the class that would have caught #3414
- [ ] No masking assertions outside the settings modal (F7 unguarded)
- [ ] Mutation-failure feedback pinned only for route-create; no
update/delete/network-level cases
- [ ] `validation.zOneOf-single-field.spec.ts` only tests the case an
inverted implementation also passes — needs both/neither quadrants
- [ ] No i18n key-parity unit test; the lang-switch integration spec
self-skips if the switcher isn't found by accessible name
- [ ] Chromium-only; e2e backend tracks the moving `apache/apisix:dev` tag
even for PR runs
- [ ] 15 `waitForTimeout` call sites; suite is parallel-unsafe locally
(specs `deleteAll*` each other's fixtures; green in CI only because `workers:
1`)
I'll be sending a first batch of small PRs for the top error-handling items
and #3412/#3296/#3414. Happy to split any checkbox above into its own issue on
request.
--
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]
