From fdd2350c55f9afe29048af6c84f5c0989859eff9 Mon Sep 17 00:00:00 2001 From: Yang Zhang Date: Wed, 1 Jul 2026 17:18:26 -0700 Subject: [PATCH] fix(frontend): keep integer property fields editable by replacing the crashing Formly number parser @ngx-formly/core's json-schema integer/number parser reads document.querySelector('#'+field.id).validity.badInput, assuming the id is on the native . ng-zorro's nz-input-number puts the id on its host element, which has no .validity, so the parser throws 'Cannot read properties of undefined (reading badInput)' whenever the parsed value is null (an intermediate state while typing). Typed edits then never commit and the field is stuck (only the +/- steppers work). Replace the numeric fields' parser in jsonSchemaMapIntercept with a null-safe parser that does no DOM access. --- .../numeric-input-parser.util.spec.ts | 293 ++++++++++++++++++ .../numeric-input-parser.util.ts | 64 ++++ .../operator-property-edit-frame.component.ts | 10 + 3 files changed, 367 insertions(+) create mode 100644 frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts create mode 100644 frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts new file mode 100644 index 00000000000..52b9d19ed99 --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.spec.ts @@ -0,0 +1,293 @@ +/** + * 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 { FormlyFieldConfig } from "@ngx-formly/core"; +import { FormlyJsonschema } from "@ngx-formly/core/json-schema"; +import { applySafeNumericParser, parseNumericInput } from "./numeric-input-parser.util"; + +describe("parseNumericInput", () => { + it("returns the number for a numeric value", () => { + expect(parseNumericInput(10)).toEqual(10); + expect(parseNumericInput(0)).toEqual(0); + expect(parseNumericInput(-5)).toEqual(-5); + }); + + it("parses numeric strings", () => { + expect(parseNumericInput("10")).toEqual(10); + expect(parseNumericInput("-5")).toEqual(-5); + expect(parseNumericInput("0")).toEqual(0); + expect(parseNumericInput("3.5")).toEqual(3.5); + }); + + it("treats empty / null / undefined as null", () => { + expect(parseNumericInput("")).toBeNull(); + expect(parseNumericInput(null)).toBeNull(); + expect(parseNumericInput(undefined)).toBeNull(); + }); + + it("returns null for non-numeric input instead of throwing", () => { + expect(parseNumericInput("abc")).toBeNull(); + expect(parseNumericInput("1a")).toBeNull(); + expect(parseNumericInput({})).toBeNull(); + }); + + it("preserves negative and zero values (does not clamp to 1)", () => { + // Guards the reported bug: a negative limit must pass through unchanged here, + // not be turned into 1 or dropped. + expect(parseNumericInput(-100)).toEqual(-100); + expect(parseNumericInput("-1")).toEqual(-1); + }); + + it("parses negative decimals and whitespace-padded numbers", () => { + expect(parseNumericInput("-3.5")).toEqual(-3.5); + expect(parseNumericInput(" 10 ")).toEqual(10); + }); + + it("treats whitespace-only strings as null", () => { + expect(parseNumericInput(" ")).toBeNull(); + expect(parseNumericInput(" ")).toBeNull(); + expect(parseNumericInput("\t")).toBeNull(); + }); + + it("rejects NaN and infinities as null", () => { + expect(parseNumericInput(NaN)).toBeNull(); + expect(parseNumericInput(Infinity)).toBeNull(); + expect(parseNumericInput(-Infinity)).toBeNull(); + expect(parseNumericInput("Infinity")).toBeNull(); + }); + + it("rejects non-number/non-string values (boolean, array, object) as null", () => { + expect(parseNumericInput(true)).toBeNull(); + expect(parseNumericInput(false)).toBeNull(); + expect(parseNumericInput([5])).toBeNull(); + expect(parseNumericInput([1, 2])).toBeNull(); + }); + + it("preserves large finite numbers", () => { + expect(parseNumericInput(1_000_000_000)).toEqual(1_000_000_000); + expect(parseNumericInput("2000000")).toEqual(2000000); + }); + + it("never touches the DOM (safe to call without an element)", () => { + // The whole point of the replacement: no document.querySelector / .validity access. + expect(() => parseNumericInput("42")).not.toThrow(); + expect(parseNumericInput("42")).toEqual(42); + }); +}); + +describe("applySafeNumericParser", () => { + it("replaces a numeric field's existing (crash-prone) parser with the safe one", () => { + // Mimics @ngx-formly's parser that throws on nz-input-number when the value is null. + const crashingParser = () => { + throw new TypeError("Cannot read properties of undefined (reading 'badInput')"); + }; + const field: FormlyFieldConfig = { type: "integer", parsers: [crashingParser] }; + + applySafeNumericParser(field); + + // the crash-prone parser is gone + expect(field.parsers).toHaveLength(1); + expect(field.parsers?.[0]).not.toBe(crashingParser); + // the null intermediate state that used to crash is now handled safely + expect(() => (field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)(null, field)).not.toThrow(); + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)(null, field)).toBeNull(); + // and it still converts real values + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("10", field)).toEqual(10); + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("-5", field)).toEqual(-5); + }); + + it("replaces the parser for type 'number' too (not just 'integer')", () => { + const field: FormlyFieldConfig = { + type: "number", + parsers: [ + () => { + throw new Error("original numeric parser should not run"); + }, + ], + }; + + applySafeNumericParser(field); + + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("3.5", field)).toEqual(3.5); + }); + + it("does NOT touch an enum (select) field's parser", () => { + // Attribute selectors are 'enum' fields; they carry a (string) parser but render as a + // select, not nz-input-number, so they must be left alone. + const enumParser = (v: unknown) => v; + const field: FormlyFieldConfig = { type: "enum", parsers: [enumParser] }; + + applySafeNumericParser(field); + + expect(field.parsers?.[0]).toBe(enumParser); + }); + + it("does NOT touch a string field's parser (regression: text inputs must keep working)", () => { + // FormlyJsonschema also sets a parser on string fields. Replacing it with the numeric + // parser would turn typed text into null and break every text property across all + // operators, so string fields must be left completely alone. + const stringParser = (v: unknown) => v; + const field: FormlyFieldConfig = { type: "string", parsers: [stringParser] }; + + applySafeNumericParser(field); + + expect(field.parsers).toHaveLength(1); + expect(field.parsers?.[0]).toBe(stringParser); // exact same parser, untouched + // typed text still passes through unchanged + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("hello", field)).toEqual("hello"); + }); + + it("leaves non-numeric fields (without parsers) untouched", () => { + const field: FormlyFieldConfig = { type: "string" }; + + applySafeNumericParser(field); + + expect(field.parsers).toBeUndefined(); + }); + + it("does not add parsers to a field that had none", () => { + const field: FormlyFieldConfig = { key: "someBoolean", type: "boolean" }; + + applySafeNumericParser(field); + + expect(field.parsers).toBeUndefined(); + }); + + it("the replacement parser treats the clear-field cases ('' and null) as null", () => { + const field: FormlyFieldConfig = { + type: "integer", + parsers: [ + () => { + throw new Error("original parser should not run"); + }, + ], + }; + + applySafeNumericParser(field); + const parser = field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown; + + expect(parser("", field)).toBeNull(); + expect(parser(null, field)).toBeNull(); + }); + + it("collapses multiple existing parsers into a single safe one", () => { + const field: FormlyFieldConfig = { type: "integer", parsers: [() => 1, () => 2] }; + + applySafeNumericParser(field); + + expect(field.parsers).toHaveLength(1); + expect((field.parsers?.[0] as (v: unknown, f: FormlyFieldConfig) => unknown)("7", field)).toEqual(7); + }); + + it("leaves an empty parsers array untouched (nothing to replace)", () => { + const field: FormlyFieldConfig = { type: "integer", parsers: [] }; + + applySafeNumericParser(field); + + expect(field.parsers).toEqual([]); + }); +}); + +describe("applySafeNumericParser with the real FormlyJsonschema (integration)", () => { + // Uses the actual @ngx-formly/core json-schema conversion (not hand-built fields) to + // prove, against real library behavior, that only numeric fields are affected. This is + // the exact concern behind the earlier "all operators can't input values" regression: + // FormlyJsonschema sets a parser on BOTH string and number fields, so the guard must + // touch only the numeric one. + const jsonSchema = new FormlyJsonschema(); + + it("replaces a real integer field's parser but leaves a real string field's parser intact", () => { + const stringField = jsonSchema.toFieldConfig({ type: "string" }); + const integerField = jsonSchema.toFieldConfig({ type: "integer" }); + + // premise: the library really does set parsers on both, and the types are as expected + expect(stringField.type).toEqual("string"); + expect(integerField.type).toEqual("integer"); + expect((stringField.parsers ?? []).length).toBeGreaterThan(0); + expect((integerField.parsers ?? []).length).toBeGreaterThan(0); + + const originalStringParser = stringField.parsers![0]; + + applySafeNumericParser(stringField); + applySafeNumericParser(integerField); + + // the string field is completely untouched → text still passes through unchanged, + // i.e. every text property on every operator keeps working + expect(stringField.parsers![0]).toBe(originalStringParser); + expect( + (stringField.parsers![0] as (v: unknown, f: FormlyFieldConfig) => unknown)("hello world", stringField) + ).toEqual("hello world"); + + // the integer field is switched to the safe numeric parser + expect((integerField.parsers![0] as (v: unknown, f: FormlyFieldConfig) => unknown)("10", integerField)).toEqual(10); + expect((integerField.parsers![0] as (v: unknown, f: FormlyFieldConfig) => unknown)(null, integerField)).toBeNull(); + }); + + it("leaves a real string field WITH an enum (attribute-selector style) intact", () => { + // An attribute selector is a string schema with an enum → the library makes it type + // 'enum'; its parser (set during the string phase) must not be swapped. + const enumField = jsonSchema.toFieldConfig({ type: "string", enum: ["a", "b", "c"] }); + expect(enumField.type).toEqual("enum"); + const originalParser = enumField.parsers?.[0]; + + applySafeNumericParser(enumField); + + expect(enumField.parsers?.[0]).toBe(originalParser); + }); + + it("invariant across every JSON-schema field type: only number/integer are changed", () => { + // Operators are just combinations of these property types, so proving the invariant + // per type proves it for all (hundreds of) operators: applySafeNumericParser must + // change field.parsers ONLY for number/integer, and leave the parsers reference + // byte-for-byte identical for every other type. + const schemas: Record = { + string: { type: "string" }, + "string+minLength": { type: "string", minLength: 2 }, + "string+pattern": { type: "string", pattern: "^a" }, + "string+enum": { type: "string", enum: ["a", "b"] }, + "nullable-string": { type: ["string", "null"] }, + boolean: { type: "boolean" }, + array: { type: "array", items: { type: "string" } }, + "array-of-numbers": { type: "array", items: { type: "integer" } }, + object: { type: "object", properties: { x: { type: "string" } } }, + const: { const: "fixed" }, + integer: { type: "integer" }, + number: { type: "number" }, + "nullable-integer": { type: ["integer", "null"] }, + }; + + for (const [name, schema] of Object.entries(schemas)) { + const field = jsonSchema.toFieldConfig(schema as never); + const parsersBefore = field.parsers; + const isNumeric = field.type === "number" || field.type === "integer"; + + applySafeNumericParser(field); + + if (isNumeric) { + // numeric fields get the safe parser (when the library gave them one) + if (parsersBefore && parsersBefore.length > 0) { + expect(field.parsers?.[0], `numeric type "${name}" should be replaced`).not.toBe(parsersBefore[0]); + } + } else { + // every non-numeric type keeps the exact same parsers reference (fully untouched) + expect(field.parsers, `non-numeric type "${name}" must be untouched`).toBe(parsersBefore); + } + } + }); +}); diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts new file mode 100644 index 00000000000..d7bfcc53503 --- /dev/null +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/numeric-input-parser.util.ts @@ -0,0 +1,64 @@ +/** + * 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 { FormlyFieldConfig } from "@ngx-formly/core"; + +/** + * Null-safe parser for numeric (integer/number) property fields. + * + * It replaces @ngx-formly/core's json-schema integer/number parser, which reads + * `document.querySelector('#' + field.id).validity.badInput`. That code assumes the + * field id is on the native ``, but ng-zorro's `nz-input-number` puts the id + * on its host element (which has no `.validity`), so the parser throws + * "Cannot read properties of undefined (reading 'badInput')" and typed edits never + * commit. This version does no DOM access: it just converts the value to a number, or + * null when it is empty/undefined/not a number. + */ +export function parseNumericInput(value: unknown): number | null { + if (typeof value === "number") { + // Reject NaN and ±Infinity — only real, finite numbers are valid. + return Number.isFinite(value) ? value : null; + } + if (typeof value === "string") { + const trimmed = value.trim(); + if (trimmed === "") { + return null; + } + const parsed = Number(trimmed); + return Number.isFinite(parsed) ? parsed : null; + } + // null, undefined, boolean, object, array, etc. are not valid numeric input. + return null; +} + +/** + * Replace a numeric field's crash-prone Formly parser with {@link parseNumericInput}. + * + * FormlyJsonschema sets `parsers` on integer/number fields (the crash-prone one) but + * ALSO on string fields (a different, harmless one). Only the numeric parser causes the + * nz-input-number crash, so restrict the swap to number/integer fields — replacing a + * string field's parser with a numeric one would drop all typed text and break every + * text property. Leave every other field untouched. + */ +export function applySafeNumericParser(field: FormlyFieldConfig): void { + const isNumericField = field.type === "number" || field.type === "integer"; + if (isNumericField && field.parsers && field.parsers.length > 0) { + field.parsers = [value => parseNumericInput(value)]; + } +} diff --git a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts index 2512fecdacb..38b5bde66e6 100644 --- a/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts +++ b/frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts @@ -74,6 +74,7 @@ import { WorkflowPveService } from "../../../service/virtual-environment/virtual import { ComputingUnitStatusService } from "../../../../common/service/computing-unit/computing-unit-status/computing-unit-status.service"; import { of } from "rxjs"; import { map, switchMap, take } from "rxjs/operators"; +import { applySafeNumericParser } from "./numeric-input-parser.util"; Quill.register("modules/cursors", QuillCursors); @@ -488,6 +489,15 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On }, }; + // @ngx-formly/core's json-schema parser for integer/number fields reads + // document.querySelector('#' + field.id).validity.badInput, assuming the id is on + // the native . ng-zorro's nz-input-number puts the id on its host element + // (which has no `.validity`), so that parser throws + // "Cannot read properties of undefined (reading 'badInput')" and typed edits never + // commit — the field gets stuck (only the +/- steppers work). FormlyJsonschema only + // sets `parsers` for numeric fields, so replace it with a null-safe parser here. + applySafeNumericParser(mappedField); + // Disable dummy operator for user if (mappedField.key === "dummyOperator") { mappedField.expressions = {