Skip to content

Commit f598601

Browse files
authored
fix(frontend): remove React 19 compatibility shim (#13264)
* chore(frontend): remove React 19 compatibility shim Signed-off-by: Jeff Spahr <spahrj@gmail.com> * fix(frontend): normalize mock query param typing Signed-off-by: Jeff Spahr <spahrj@gmail.com> * style(frontend): fix formatting for CI Signed-off-by: Jeff Spahr <spahrj@gmail.com> * fix(frontend): harden mock query decoding Signed-off-by: Jeff Spahr <spahrj@gmail.com> * docs(frontend): refresh React 19 checklist follow-up note Signed-off-by: Jeff Spahr <spahrj@gmail.com> * refactor(frontend): remove redundant mock query extraction Signed-off-by: Jeff Spahr <spahrj@gmail.com> --------- Signed-off-by: Jeff Spahr <spahrj@gmail.com>
1 parent 34c4ea0 commit f598601

86 files changed

Lines changed: 346 additions & 183 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

frontend/docs/react-18-19-upgrade-checklist.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@
2727

2828
**Current focus**:
2929

30-
- The React 18/19 upgrade track is complete through `#15`.
31-
- Remaining React 19 follow-up cleanup is limited to removing the temporary `frontend/src/react19-compat.d.ts` shim noted during `#13`.
30+
- The React 18/19 upgrade track is complete through `#15`, including the smaller React 19 follow-up cleanup items noted during `#13`.
3231

3332
**How to contribute**: This checklist is now complete. Follow-up frontend work should track new issues directly and continue to pass `npm run test:ci` and `npm run build` before merge.
3433

@@ -335,9 +334,7 @@ Run `npm run check:react-peers` (targeting React 19), upgrade any remaining Reac
335334
Completed by [#13153](https://github.com/kubeflow/pipelines/pull/13153), merged on March 27, 2026 UTC. `frontend/package.json` now pins `react`, `react-dom`, `@types/react`, and `@types/react-dom` to `^19`, and the default React peer gate target is React 19.
336335

337336
**Current note**:
338-
The React 19 core bump is complete. One smaller follow-up cleanup item remains from the review thread:
339-
340-
- remove the temporary `frontend/src/react19-compat.d.ts` shim by converting remaining `JSX.Element`-style annotations to `React.JSX.*` or inferred types
337+
The React 19 core bump and its smaller follow-up review-thread cleanup items are complete.
341338

342339
**Description** (historical):
343340
Bump `react`, `react-dom`, `@types/react`, and `@types/react-dom` to v19, address any React 19-specific breakages, regenerate and review affected snapshots, and clear the temporary React core allowlist from the `#12` sweep.

frontend/mock-backend/mock-api-middleware.ts

Lines changed: 119 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,64 @@ interface BaseResource {
6262
error?: string;
6363
}
6464

65+
function getQueryString(queryParam: unknown): string | undefined {
66+
if (typeof queryParam === 'string') {
67+
return queryParam;
68+
}
69+
if (Array.isArray(queryParam)) {
70+
return queryParam.find((value): value is string => typeof value === 'string');
71+
}
72+
return undefined;
73+
}
74+
75+
function getQueryNumber(queryParam: unknown): number | undefined {
76+
const queryString = getQueryString(queryParam);
77+
if (queryString === undefined || queryString === '') {
78+
return undefined;
79+
}
80+
const queryNumber = Number(queryString);
81+
return Number.isNaN(queryNumber) ? undefined : queryNumber;
82+
}
83+
84+
function getDecodedQueryString(queryParam: unknown): string | undefined {
85+
const queryString = getQueryString(queryParam);
86+
if (queryString === undefined) {
87+
return undefined;
88+
}
89+
try {
90+
return decodeURIComponent(queryString);
91+
} catch {
92+
return undefined;
93+
}
94+
}
95+
96+
function getRequiredDecodedQueryString(
97+
res: Response,
98+
queryParam: unknown,
99+
queryParamName: string,
100+
): string | undefined {
101+
const queryString = getQueryString(queryParam);
102+
if (queryString === undefined || queryString === '') {
103+
res.status(400).send(`${queryParamName} argument is required`);
104+
return undefined;
105+
}
106+
107+
let decodedQueryString: string;
108+
try {
109+
decodedQueryString = decodeURIComponent(queryString);
110+
} catch {
111+
res.status(400).send(`${queryParamName} argument is invalid`);
112+
return undefined;
113+
}
114+
115+
if (decodedQueryString === '') {
116+
res.status(400).send(`${queryParamName} argument is required`);
117+
return undefined;
118+
}
119+
120+
return decodedQueryString;
121+
}
122+
65123
// tslint:disable-next-line:no-default-export
66124
export default (app: express.Application) => {
67125
app.use((req, _, next) => {
@@ -122,11 +180,15 @@ export default (app: express.Application) => {
122180
};
123181

124182
let jobs: ApiJob[] = fixedData.jobs;
125-
if (req.query.filter) {
126-
jobs = filterResources(fixedData.jobs, req.query.filter);
183+
const filterQuery = getQueryString(req.query.filter);
184+
if (filterQuery) {
185+
jobs = filterResources(fixedData.jobs, filterQuery);
127186
}
128187

129-
const { desc, key } = getSortKeyAndOrder(ExperimentSortKeys.CREATED_AT, req.query.sort_by);
188+
const { desc, key } = getSortKeyAndOrder(
189+
ExperimentSortKeys.CREATED_AT,
190+
getQueryString(req.query.sort_by),
191+
);
130192

131193
jobs.sort((a, b) => {
132194
let result = 1;
@@ -139,8 +201,8 @@ export default (app: express.Application) => {
139201
return result * (desc ? -1 : 1);
140202
});
141203

142-
const start = req.query.page_token ? +req.query.page_token : 0;
143-
const end = start + (+req.query.page_size || 20);
204+
const start = getQueryNumber(req.query.page_token) || 0;
205+
const end = start + (getQueryNumber(req.query.page_size) || 20);
144206
response.jobs = jobs.slice(start, end);
145207

146208
if (end < jobs.length) {
@@ -159,11 +221,15 @@ export default (app: express.Application) => {
159221
};
160222

161223
let experiments: ApiExperiment[] = fixedData.experiments;
162-
if (req.query.filter) {
163-
experiments = filterResources(fixedData.experiments, req.query.filter);
224+
const filterQuery = getQueryString(req.query.filter);
225+
if (filterQuery) {
226+
experiments = filterResources(fixedData.experiments, filterQuery);
164227
}
165228

166-
const { desc, key } = getSortKeyAndOrder(ExperimentSortKeys.NAME, req.query.sortBy);
229+
const { desc, key } = getSortKeyAndOrder(
230+
ExperimentSortKeys.NAME,
231+
getQueryString(req.query.sortBy),
232+
);
167233

168234
experiments.sort((a, b) => {
169235
let result = 1;
@@ -176,8 +242,8 @@ export default (app: express.Application) => {
176242
return result * (desc ? -1 : 1);
177243
});
178244

179-
const start = req.query.pageToken ? +req.query.pageToken : 0;
180-
const end = start + (+req.query.pageSize || 20);
245+
const start = getQueryNumber(req.query.pageToken) || 0;
246+
const end = start + (getQueryNumber(req.query.pageSize) || 20);
181247
response.experiments = experiments.slice(start, end);
182248

183249
if (end < experiments.length) {
@@ -275,21 +341,25 @@ export default (app: express.Application) => {
275341

276342
let runs: ApiRun[] = fixedData.runs.map((r) => r.run!);
277343

278-
if (req.query.filter) {
279-
runs = filterResources(runs, req.query.filter);
344+
const filterQuery = getQueryString(req.query.filter);
345+
if (filterQuery) {
346+
runs = filterResources(runs, filterQuery);
280347
}
281348

282-
if (req.query['resource_reference_key.type'] === ApiResourceType.EXPERIMENT) {
349+
const resourceReferenceType = getQueryString(req.query['resource_reference_key.type']);
350+
const resourceReferenceId = getQueryString(req.query['resource_reference_key.id']);
351+
if (resourceReferenceType === ApiResourceType.EXPERIMENT && resourceReferenceId) {
283352
runs = runs.filter((r) =>
284353
RunUtils.getAllExperimentReferences(r).some(
285-
(ref) =>
286-
(ref.key && ref.key.id && ref.key.id === req.query['resource_reference_key.id']) ||
287-
false,
354+
(ref) => (ref.key && ref.key.id && ref.key.id === resourceReferenceId) || false,
288355
),
289356
);
290357
}
291358

292-
const { desc, key } = getSortKeyAndOrder(RunSortKeys.CREATED_AT, req.query.sort_by);
359+
const { desc, key } = getSortKeyAndOrder(
360+
RunSortKeys.CREATED_AT,
361+
getQueryString(req.query.sort_by),
362+
);
293363

294364
runs.sort((a, b) => {
295365
let result = 1;
@@ -302,8 +372,8 @@ export default (app: express.Application) => {
302372
return result * (desc ? -1 : 1);
303373
});
304374

305-
const start = req.query.page_token ? +req.query.page_token : 0;
306-
const end = start + (+req.query.page_size || 20);
375+
const start = getQueryNumber(req.query.page_token) || 0;
376+
const end = start + (getQueryNumber(req.query.page_size) || 20);
307377
response.runs = runs.slice(start, end);
308378

309379
if (end < runs.length) {
@@ -446,11 +516,15 @@ export default (app: express.Application) => {
446516
};
447517

448518
let pipelines: ApiPipeline[] = fixedData.pipelines;
449-
if (req.query.filter) {
450-
pipelines = filterResources(fixedData.pipelines, req.query.filter);
519+
const filterQuery = getQueryString(req.query.filter);
520+
if (filterQuery) {
521+
pipelines = filterResources(fixedData.pipelines, filterQuery);
451522
}
452523

453-
const { desc, key } = getSortKeyAndOrder(PipelineSortKeys.CREATED_AT, req.query.sort_by);
524+
const { desc, key } = getSortKeyAndOrder(
525+
PipelineSortKeys.CREATED_AT,
526+
getQueryString(req.query.sort_by),
527+
);
454528

455529
pipelines.sort((a, b) => {
456530
let result = 1;
@@ -463,8 +537,8 @@ export default (app: express.Application) => {
463537
return result * (desc ? -1 : 1);
464538
});
465539

466-
const start = req.query.page_token ? +req.query.page_token : 0;
467-
const end = start + (+req.query.page_size || 20);
540+
const start = getQueryNumber(req.query.page_token) || 0;
541+
const end = start + (getQueryNumber(req.query.page_size) || 20);
468542
response.pipelines = pipelines.slice(start, end);
469543

470544
if (end < pipelines.length) {
@@ -567,21 +641,19 @@ export default (app: express.Application) => {
567641
// page_size: '50',
568642
// sort_by: 'created_at desc'
569643
// },
570-
if (
571-
req.query['resource_key.id'] &&
572-
req.query['resource_key.type'] === 'PIPELINE' &&
573-
req.query.page_size > 0
574-
) {
644+
const resourceKeyId = getQueryString(req.query['resource_key.id']);
645+
const resourceKeyType = getQueryString(req.query['resource_key.type']);
646+
const pageSize = getQueryNumber(req.query.page_size);
647+
if (resourceKeyId && resourceKeyType === 'PIPELINE' && (pageSize || 0) > 0) {
575648
const response: ApiListPipelineVersionsResponse = {
576649
next_page_token: '',
577650
versions: [],
578651
};
579652

580-
let versions: ApiPipelineVersion[] =
581-
PIPELINE_VERSIONS_LIST_MAP.get(req.query['resource_key.id']) || [];
653+
let versions: ApiPipelineVersion[] = PIPELINE_VERSIONS_LIST_MAP.get(resourceKeyId) || [];
582654

583655
if (versions.length === 0) {
584-
const pipeline = fixedData.pipelines.find((p) => p.id === req.query['resource_key.id']);
656+
const pipeline = fixedData.pipelines.find((p) => p.id === resourceKeyId);
585657

586658
if (pipeline == null || !pipeline.default_version) {
587659
return;
@@ -596,8 +668,8 @@ export default (app: express.Application) => {
596668
return;
597669
}
598670

599-
const start = req.query.page_token ? +req.query.page_token : 0;
600-
const end = start + (+req.query.page_size || 20);
671+
const start = getQueryNumber(req.query.page_token) || 0;
672+
const end = start + (pageSize || 20);
601673
response.versions = versions.slice(start, end);
602674

603675
if (end < versions.length) {
@@ -666,11 +738,18 @@ export default (app: express.Application) => {
666738
});
667739

668740
app.post(v1beta1Prefix + '/pipelines/upload', (req, res) => {
669-
mockCreatePipeline(res, decodeURIComponent(req.query.name), req.body);
741+
const pipelineName = getRequiredDecodedQueryString(res, req.query.name, 'name');
742+
if (!pipelineName) {
743+
return;
744+
}
745+
mockCreatePipeline(res, pipelineName, req.body);
670746
});
671747

672748
app.get('/artifacts/get', (req, res) => {
673-
const key = decodeURIComponent(req.query.key);
749+
const key = getRequiredDecodedQueryString(res, req.query.key, 'key');
750+
if (!key) {
751+
return;
752+
}
674753
res.header('Content-Type', 'application/json');
675754
if (key.endsWith('roc.csv')) {
676755
res.sendFile(_path.resolve(__dirname, rocDataPath));
@@ -710,7 +789,10 @@ export default (app: express.Application) => {
710789
});
711790

712791
app.get('/k8s/pod/logs', (req, res) => {
713-
const podName = decodeURIComponent(req.query.podname);
792+
const podName = getRequiredDecodedQueryString(res, req.query.podname, 'podname');
793+
if (!podName) {
794+
return;
795+
}
714796
if (podName === 'json-12abc') {
715797
res.status(404).send('pod not found');
716798
return;

frontend/mock-backend/tsconfig.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@
2424
"skipLibCheck": true,
2525
"esModuleInterop": true,
2626
},
27-
"include": [".", "../src/react19-compat.d.ts"],
27+
"include": ["."],
2828
"exclude": ["dist", "coverage"],
2929
}
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// Copyright 2026 The Kubeflow Authors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
16+
import express from 'express';
17+
import path from 'path';
18+
import { fileURLToPath } from 'url';
19+
import requests from 'supertest';
20+
21+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
22+
const frontendRoot = path.resolve(__dirname, '..', '..');
23+
const originalCwd = process.cwd();
24+
25+
async function createRequest(): Promise<ReturnType<typeof requests>> {
26+
vi.resetModules();
27+
const { default: mockApiMiddleware } = await import('../../mock-backend/mock-api-middleware.ts');
28+
const app = express();
29+
mockApiMiddleware(app as any);
30+
return requests(app);
31+
}
32+
33+
beforeAll(() => {
34+
process.chdir(frontendRoot);
35+
});
36+
37+
afterAll(() => {
38+
process.chdir(originalCwd);
39+
});
40+
41+
describe('mock backend decoded query validation', () => {
42+
let request: ReturnType<typeof requests>;
43+
44+
beforeEach(async () => {
45+
vi.restoreAllMocks();
46+
vi.spyOn(console, 'info').mockImplementation(() => {});
47+
vi.spyOn(console, 'log').mockImplementation(() => {});
48+
request = await createRequest();
49+
});
50+
51+
it('rejects pipeline upload requests without a name query param', async () => {
52+
await request
53+
.post('/apis/v1beta1/pipelines/upload')
54+
.send({ uploaded: true })
55+
.expect(400)
56+
.expect('name argument is required');
57+
});
58+
59+
it('rejects pipeline upload requests with invalid percent-encoding in the name query param', async () => {
60+
await request
61+
.post('/apis/v1beta1/pipelines/upload?name=%E0%A4%A')
62+
.send({ uploaded: true })
63+
.expect(400)
64+
.expect('name argument is invalid');
65+
});
66+
67+
it('rejects artifact requests without a key query param', async () => {
68+
await request.get('/artifacts/get').expect(400).expect('key argument is required');
69+
});
70+
71+
it('rejects artifact requests with invalid percent-encoding in the key query param', async () => {
72+
await request.get('/artifacts/get?key=%E0%A4%A').expect(400).expect('key argument is invalid');
73+
});
74+
75+
it('rejects pod log requests without a podname query param', async () => {
76+
await request.get('/k8s/pod/logs').expect(400).expect('podname argument is required');
77+
});
78+
79+
it('rejects pod log requests with invalid percent-encoding in the podname query param', async () => {
80+
await request
81+
.get('/k8s/pod/logs?podname=%E0%A4%A')
82+
.expect(400)
83+
.expect('podname argument is invalid');
84+
});
85+
});

0 commit comments

Comments
 (0)