Yicong-Huang commented on PR #5676:
URL: https://github.com/apache/texera/pull/5676#issuecomment-4737371055
@rbelavadi I think the issue is a bit more complex, if I remember correctly.
It is not only a frontend issue, but also related to the contract between the
frontend property model and the backend operator property.
The purpose of frontend validation is to act as a quick guard: we want to
warn users early when some field is invalid, instead of letting them submit the
workflow and only then hit a compilation or execution error. This is also why
the run button is disabled when the frontend validator detects an error.
The bug here is related to optional fields in the operator property editor.
Once a user enters a value, the value cannot be removed because `null` is
treated as invalid. The root cause is that Formly, which we use to render the
operator property panel, removes a value by setting it to `null`. Formly’s
internal `model` is a JSON object, and validation is performed against that
JSON.
For example, suppose we have the following value, where `y` is optional:
```ts
{ x: 10, y: 20 }
```
Removing `y` currently results in:
```ts
{ x: 10, y: null }
```
which Formly then marks as invalid.
However, if the resulting model were:
```ts
{ x: 10 }
```
then it should be considered valid, since `y` is optional.
For this particular case, the field is an enum rendered as a dropdown menu.
So I think we need a way to “deselect/clear” the enum value. When a real enum
value is selected, we should still validate it normally. But when the deselect
option is chosen, ideally we should remove the field from the resulting JSON
model instead of setting it to `null`.
So regarding your solution, I think it allows the validator to not error out
on a `null` value. That may fix this symptom, but it could also weaken the
frontend quick validation if `null` is not actually a valid value accepted by
the backend. We should think carefully about whether allowing optional fields
to be passed as `null` is an acceptable frontend-backend protocol. My intuition
is that removing the field from the JSON model would be the cleaner fix.
--
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]
