fix: a.trim is not a function

[DSingh0304]

Why submit this pull request?

• Bugfix
• New feature provided
• Improve performance
• Backport patches

What changes will this PR take into?

This PR fixes a runtime error "a.trim is not a function" that occurs when viewing a route with a vars field in the detail page.

The issue was caused by the TagsInput component (from Mantine) receiving non-string values (arrays) in some edge cases. The Mantine TagsInput internally calls .trim() on each value, which fails when the value is not a string.

Changes made:

• Added a defensive check in src/components/form/TagInput.tsx to ensure the value passed to TagsInput is always an array of strings
• Non-string values are now filtered out before being passed to the component

Related issues

fix #3276

Checklist:

• Did you explain what problem does this PR solve? Or what new features have been added?
• Have you added corresponding test cases?
• Have you modified the corresponding document?
• Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

[Baoyuantop]

Hi @DSingh0304, could you add a test to fix this issue?

[DSingh0304]

Hi @DSingh0304, could you add a test to fix this issue?

Yes sure I will..

[DSingh0304]

Hi @DSingh0304, could you add a test to fix this issue?

The fix is defensive:

• It prevents crashes if corrupted data somehow reaches the frontend
• The existing e2e test routes.crud all fields.spec.ts already tests the vars field with valid data.
• Since APISIX API validates data strictly, invalid data can't be created via the API for e2e testing.
• The project doesn't have unit test infrastructure for component-level testing.

However, if you'd like me to add a test, I can create an e2e test that mocks the API response to simulate corrupted data reaching the frontend. This would demonstrate that the components handle invalid data gracefully, though it would deviate from the existing test pattern where all tests use real API calls for creation and deletion.

[eccles]

Hi everyone

Hate to nag but is there any chance of progressing this change?
I am not a ts dev but I could possibly help test this in our local deployment (using docker-compose)

[Baoyuantop]

Hi @DSingh0304, could you please take a look at the reproduction steps in the original issue? It seems that @eccles suggested calling the admin API to create this error data.

[DSingh0304]

Hi @DSingh0304, could you please take a look at the reproduction steps in the original issue? It seems that @eccles suggested calling the admin API to create this error data.

Hi @Baoyuantop, thanks for the clarification! You're right. I misunderstood the scenario earlier. The vars data in the original issue is completely valid APISIX data, not corrupted data. The issue is that it was created via the Admin API, and the dashboard UI couldn't display it properly.

I'll add an E2E test

[DSingh0304]

Hey @Baoyuantop I have added the test...

[Copilot]

Pull request overview

Fixes a runtime crash (a.trim is not a function) in the dashboard route detail view by hardening form components against unexpected non-string values, and adds an E2E regression test covering routes created with vars via the Admin API (issue #3276).

Changes:

• Add defensive normalization in FormItemTagsInput to avoid passing non-strings into Mantine TagsInput.
• Add defensive guard in FormItemLabels to only process plain-object label values.
• Add Playwright E2E test to ensure viewing a route created with vars does not crash.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/components/form/TagInput.tsx	Adds value sanitization before rendering Mantine TagsInput to prevent .trim() runtime crashes.
src/components/form/Labels.tsx	Adds a guard to avoid treating non-object/array values as label maps.
e2e/tests/routes.vars-admin-api.spec.ts	New E2E test that creates a route with vars via Admin API and validates the detail view loads.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/components/form/TagInput.tsx:68

• {...restProps} is spread after the component’s controlled value/onChange, so any caller-provided value or onChange in restProps will override the react-hook-form wiring (and bypass the new defensive sanitization). Consider omitting value/onChange (and related controlled props) from the public prop type and/or moving {...restProps} before the explicit value/onChange props so the wrapper remains the source of truth.

    <TagsInput
      value={tagsInputValue}
      error={fieldState.error?.message}
      onChange={(value) => {
        const val = to ? value.map(to) : value;
        fOnChange(val);
        restProps?.onChange?.(value);
      }}
      comboboxProps={{ shadow: 'md' }}
      acceptValueOnBlur
      {...restField}
      {...restProps}
    />

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[DSingh0304]

Hi @Baoyuantop I have addressed the comments by copilot..

[DSingh0304]

I will fix this so that it passes the e2e test.

[Baoyuantop]

Hi @DSingh0304, please fix the failed test

[DSingh0304]

Hi @DSingh0304, please fix the failed test

Yes sir I will fix it asap.

[DSingh0304]

Hey @Baoyuantop I have resolved the issue which was causing the CI failure can you please run the workflow again.

[Baoyuantop]

The fix direction (defensive filtering of non-string values in TagsInput) is correct, but there are a few issues to address:

1. Unrelated changes: The large diffs in routeTree.gen.ts and apisix_conf.yml are unrelated to this bug fix. Please revert them and handle separately if needed.

2. CI status: The current mergeable_state is unstable. Please ensure the latest commit passes all required checks.

3. Silent data filtering risk: TagInput silently drops non-string items, which could cause the UI to show fewer values than actually exist. Suggestions:

   • At minimum, add a test to document this as expected behavior
   • Better approach: add an observable warning (log or UI-level) when filtering occurs

Please address these items for re-review.

[DSingh0304]

The fix direction (defensive filtering of non-string values in TagsInput) is correct, but there are a few issues to address:

1. Unrelated changes: The large diffs in routeTree.gen.ts and apisix_conf.yml are unrelated to this bug fix. Please revert them and handle separately if needed.

2. CI status: The current mergeable_state is unstable. Please ensure the latest commit passes all required checks.

3. Silent data filtering risk: TagInput silently drops non-string items, which could cause the UI to show fewer values than actually exist. Suggestions:

   • At minimum, add a test to document this as expected behavior
   • Better approach: add an observable warning (log or UI-level) when filtering occurs

Please address these items for re-review.

I will go through the comments and address each of them by today.

[DSingh0304]

@Baoyuantop I have fixed the lint errors and addressed the comments.

[DSingh0304]

@Baoyuantop All the tests pass and the changes suggested by you are also implemented.