lxbme opened a new pull request, #3439: URL: https://github.com/apache/apisix-dashboard/pull/3439
**Why submit this pull request?** - [x] Bugfix - [ ] New feature provided - [ ] Improve performance - [ ] Backport patches **What changes will this PR take into?** Part of #3417 (UX section): "Form TOC never refreshes when sections appear dynamically: `refreshTOC` builds a debounced function and discards it without calling (`FormSection/index.tsx:149-152`)." **Runtime finding first, because it corrects the issue:** the user-visible claim is *refuted*. On current master, toggling the health-check switch does add Active/Healthy/Unhealthy/Passive to the TOC, and disabling removes them (verified by driving the real page and dumping the TOC before/after). The code reading in the issue is nevertheless accurate — `refreshTOC` really does build a debounced wrapper on every call and discard it. The refresh works **by accident**: the discarded function is the return value of the `useShallowEffect` callback, so React stores it as the effect **cleanup** and invokes it on the next children change or unmount — one change behind the intent, masked in practice by the burst of section mounts/unmounts a toggle causes. That accident is a landmine: any future cleanup of the "pointless" return value silently kills the TOC refresh, and the wiring misleads every reader (including the #3417 reviewer, whose careful reading still predicted the wrong runtime behavior). This PR makes the accident a contract: - `FormTOCBox` creates **one** stable debounced function (`useMemo`), calling through the ref so it always reaches Mantine's latest `reinitialize` (the old code would also have captured the initial no-op ref value had it ever been invoked directly); - the `FormSection` effect now invokes `refreshTOC()` on children changes and **deliberately** returns it as the cleanup, so sections that unmount also drop out of the TOC — same observable behavior as today's accident, stated in code; - a comment records the history so the shape is not "simplified" back into a bug. Small honest note: rambdax's `debounce` types its result as `(input) => void`, which is not assignable to the context's `() => void` — hence the zero-arg wrapper inside the `useMemo`. **Tests.** The new spec `form.toc-refresh-on-dynamic-sections.spec.ts` is a **characterization test, not a red/green regression**: it passes both before and after this change (the behavior already worked, accidentally). It pins both directions — enabling health checks must add the section entries to the TOC, disabling must remove them — so any refactor that breaks the refresh path fails loudly instead of silently. Blast radius (`FormSection` is shared by every form page): full local e2e suite — **177 passed**; the 2 failures are documented environment items unrelated to this change (a bulk-page pagination load flake, green on isolated rerun, and `stream_routes.show-disabled-error`, which cannot run outside the repo's own compose project). Lint and build clean. **Related issues** Part of #3417 (please do not auto-close the tracking issue) **Checklist:** - [x] Did you explain what problem does this PR solve? Or what new features have been added? - [x] Have you added corresponding test cases? - [ ] Have you modified the corresponding document? (no user-facing document covers the form TOC) - [x] Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first -- 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]
