DSingh0304 commented on PR #3274:
URL:
https://github.com/apache/apisix-dashboard/pull/3274#issuecomment-4249428482
> Thank you for the significant effort on the SchemaForm component. The
component architecture is well-designed with a clean recursive rendering
pattern, and the test coverage is quite thorough (50 tests). The overall
direction is solid.
>
> However, there are several blocking issues that need to be resolved before
merging:
>
> 1. Dependency downgrades: This PR downgrades several TanStack packages
to significantly older versions (e.g., react-router from ^1.168.3 to ^1.116.0).
This is likely due to developing on an older fork of master. Please rebase onto
the latest master and keep upstream dependency versions, only adding the new
dependencies required by SchemaForm (ajv, vitest, testing-library, etc.).
>
> 2. producer.ts regression: The modification to the pipeProduce
function replaces produceRestoreEmptyPlugins with a two-pass marker approach,
which drops the discovery_args: {} restoration logic. This change is unrelated
to SchemaForm and introduces a regression. Please revert all changes to
producer.ts.
>
> 3. Unrelated .gitignore entries: The added .gemini/artifacts/ and
jsonschema-poc/ entries are personal development artifacts and should not be
committed to the project's .gitignore. Please remove these entries.
>
> 4. Demo route in production: schema_form_demo is registered as a
production route and will appear in release builds. Please either remove it or
make it available only in development mode.
>
>
> Additional suggestions (non-blocking):
>
> * OneOfFields and AnyOfFields are nearly identical (~150 lines of
duplication). Consider extracting a shared VariantFields component.
>
> * findDiscriminators only recognizes const-based discriminators.
Consider extending it to support single-value enum as well.
>
> * collectAllPaths does not handle array items or patternProperties
sub-paths, which may cause missed unregistrations on branch switching.
>
> * formatLabel renders "oauth" as "OAUTH" while the standard form is
"OAuth".
Thanks for the review! @Baoyuantop The blockers are fixed:
1. **Dependencies** - Rebased onto latest master, all TanStack packages now
aligned
2. **producer.ts** - Verified `discovery_args` restoration is intact
3. **.gitignore** - Confirmed clean, no personal artifacts tracked
4. **Demo route** - Deleted `schema_form_demo.tsx` and demo utilities
I'll will make further changes in the PR so that it can be merge ready and
will update you in a while.
--
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]
