DSingh0304 commented on PR #3277:
URL:
https://github.com/apache/apisix-dashboard/pull/3277#issuecomment-4089137896
> The core fix (`emptyObjectsCleaner: false`) is correct and the simplest
approach — much better than the previous marker-based pipeline. However there
are a few issues that should be addressed before merging.
> ### `JSON.stringify` + manual Content-Type in `src/apis/upstreams.ts` —
should be reverted
>
> Axios already serializes JS objects to JSON and sets `Content-Type:
application/json` by default. Manually calling `JSON.stringify(data)` before
passing to Axios means the data is sent as a **pre-stringified string**, which
can cause double-encoding issues in certain interceptor scenarios.
>
> No other API file in this project (`routes.ts`, `services.ts`,
`consumers.ts`, `ssls.ts`, etc.) uses `JSON.stringify` — they all pass plain
objects. This change breaks consistency and is **not needed** for the fix: the
empty-object stripping happens in `pipeProduce` on the client side, not during
Axios serialization.
>
> **Suggestion:** Revert all changes to `src/apis/upstreams.ts`.
> ### `src/routes/services/add.tsx` — data cleaning logic was moved to an
inconsistent location
>
> On master, `pipeProduce(produceRmUpstreamWhenHas('upstream_id'))` lives
inside `mutationFn`. This PR splits it: `mutationFn` now receives raw data, and
the cleaning happens in `form.handleSubmit`. This breaks the established
pattern where data transformation is centralized inside the mutation.
>
> Compare with `src/routes/services/detail.$id/index.tsx` in this same PR —
the detail page correctly keeps cleaning inside `mutationFn`. The add page
should be consistent.
>
> The `as ServicePostType` cast also masks potential type mismatches.
>
> **Suggestion:** Keep the master pattern — add
`produceRmEmptyUpstreamFields` inside `mutationFn` alongside the existing
`produceRmUpstreamWhenHas`:
>
> ```ts
> mutationFn: (d: ServicePostType) =>
> postServiceReq(
> req,
> pipeProduce(produceRmUpstreamWhenHas('upstream_id'),
produceRmEmptyUpstreamFields)(d)
> ),
> ```
>
> And keep `form.handleSubmit((d) => postService.mutateAsync(d))` unchanged.
> ### `src/utils/useSearchParams.ts` change is unrelated to this PR
>
> The change types `prev` as `Record<string, unknown>` which discards
TanStack Router's inferred search-param type, and `as P & Partial<P>` is a
no-op cast (`P & Partial<P>` ≡ `P`).
>
> This is unrelated to the empty-object fix and should be removed from this
PR (or addressed in a separate one if there's an actual type error to fix).
I will go through the comment and fix the CI.
--
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]
