Copilot commented on code in PR #3286:
URL: https://github.com/apache/apisix-dashboard/pull/3286#discussion_r2954580917
##########
src/components/form/TagInput.tsx:
##########
@@ -44,9 +44,17 @@ export const FormItemTagsInput = <T extends FieldValues, R>(
field: { value, onChange: fOnChange, ...restField },
fieldState,
} = useController<T>(controllerProps);
+
+ // Defensive: ensure value is always an array of strings to prevent
+ // "a.trim is not a function" errors when non-string values are passed
+ const rawValue = Array.isArray(value) ? value : [];
+ const tagsInputValue = from
+ ? rawValue.map(from).filter((item: unknown): item is string => typeof item
=== 'string')
+ : rawValue.filter((item: unknown): item is string => typeof item ===
'string');
+
return (
<TagsInput
- value={from ? value.map(from) : value}
+ value={tagsInputValue}
error={fieldState.error?.message}
onChange={(value) => {
const val = to ? value.map(to) : value;
Review Comment:
The wrapper defines its own `onChange` (and `value`) but later spreads
`...restProps` into `<TagsInput />` (outside this hunk). Because JSX prop
precedence is last-one-wins, a caller-provided `onChange`/`value` inside
`restProps` would override the wrapper handler/value, breaking react-hook-form
updates and potentially reintroducing the non-string `.trim` runtime error this
change is trying to prevent. Consider omitting `value`/`onChange` from
`FormItemTagsInputProps` (or stripping them from `restProps` before spreading),
or move the spread earlier so the wrapper-controlled props always win.
##########
e2e/tests/routes.vars-admin-api.spec.ts:
##########
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { routesPom } from '@e2e/pom/routes';
+import { e2eReq } from '@e2e/utils/req';
+import { test } from '@e2e/utils/test';
+import { expect } from '@playwright/test';
+
+import { putRouteReq } from '@/apis/routes';
+import { API_ROUTES } from '@/config/constant';
+
+const routeId = 'test-vars-admin-api';
+
+test.beforeAll(async () => {
+ await e2eReq.delete(`${API_ROUTES}/${routeId}`).catch(() => {
+ // Ignore cleanup errors; route may not exist yet.
+ });
+});
+
+test('route with vars created via Admin API', async ({ page }) => {
+ await test.step('create route with vars via Admin API', async () => {
+ await putRouteReq(e2eReq, {
+ id: routeId,
+ name: routeId,
+ uri: '/test-vars-route',
+ methods: ['GET', 'POST'],
+ upstream: {
+ type: 'roundrobin',
+ nodes: [{ host: 'httpbin.org', port: 80, weight: 1 }],
+ },
+ vars: [
+ [
+ 'uri',
+ '~~',
+ '^/(.*)/v1beta/models/(gemini-3-pro-preview)(?::[A-Za-z0-9._-]+)?$',
+ ],
+ ],
+ });
+ });
+
+ await test.step('view route detail without error', async () => {
+ // Navigate to routes list
+ await routesPom.toIndex(page);
+ await routesPom.isIndexPage(page);
+
+ // Find and click "View on our route"
+ await page
+ .getByRole('row', { name: routeId })
+ .getByRole('button', { name: 'View' })
+ .click();
+
Review Comment:
This navigation relies on finding the route in the list table by row text,
which can be flaky if the table is paginated/sorted or other tests left
additional routes. A more robust approach is to navigate directly to the detail
route (e.g. `/routes/detail/${routeId}`) or to use a search/filter to ensure
the row is visible before clicking View.
##########
e2e/tests/routes.vars-admin-api.spec.ts:
##########
@@ -0,0 +1,78 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { routesPom } from '@e2e/pom/routes';
+import { e2eReq } from '@e2e/utils/req';
+import { test } from '@e2e/utils/test';
+import { expect } from '@playwright/test';
+
+import { putRouteReq } from '@/apis/routes';
+import { API_ROUTES } from '@/config/constant';
+
+const routeId = 'test-vars-admin-api';
+
+test.beforeAll(async () => {
+ await e2eReq.delete(`${API_ROUTES}/${routeId}`).catch(() => {
+ // Ignore cleanup errors; route may not exist yet.
+ });
+});
+
+test('route with vars created via Admin API', async ({ page }) => {
+ await test.step('create route with vars via Admin API', async () => {
+ await putRouteReq(e2eReq, {
+ id: routeId,
+ name: routeId,
+ uri: '/test-vars-route',
+ methods: ['GET', 'POST'],
+ upstream: {
+ type: 'roundrobin',
+ nodes: [{ host: 'httpbin.org', port: 80, weight: 1 }],
+ },
+ vars: [
+ [
+ 'uri',
+ '~~',
+ '^/(.*)/v1beta/models/(gemini-3-pro-preview)(?::[A-Za-z0-9._-]+)?$',
+ ],
+ ],
+ });
+ });
+
+ await test.step('view route detail without error', async () => {
+ // Navigate to routes list
+ await routesPom.toIndex(page);
+ await routesPom.isIndexPage(page);
+
+ // Find and click "View on our route"
Review Comment:
Comment typo/grammar: "View on our route" reads incorrect here; consider
changing to something like "View the route" / "View route" to match the action
being performed.
--
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]
