DataType.getType("float").isValid(NaN) Returns true

[boghyon]

URL (minimal example if possible)

https://openui5nightly.hana.ondemand.com

Steps to reproduce the problem:

1. Open the above page
2. Open browser console
3. Enter sap.ui.base.DataType.getType("int").isValid(NaN); ==> Returns false
4. Enter sap.ui.base.DataType.getType("float").isValid(NaN); ==> Returns true

What is the expected result?

While it is true that NaN is of type number in JS, allowing NaN to be valid in UI5 would also make this case valid:

<SomeControl propertyAwaitingFloat="0.02" /> ✔️
<SomeControl propertyAwaitingFloat="asdasdasd" /> ✔️😕

.. since parsing "asdasdasd" results in NaN. And typeof NaN === "number" returns true.

openui5/src/sap.ui.core/src/sap/ui/base/DataType.js

Lines 399 to 404 in 0b901f6

	isValid : function(vValue) {
	return typeof vValue === "number";
	},
	parseValue: function(sValue) {
	return parseFloat(sValue);
	}

I've had expected to get the error log implemented in 544ee13.

Value 'asdasdasd' is not valid for type 'float'.

What happens instead?

No error is thrown.

[codeworrior]

I understand that the inconsistency between int and float reg. NaN is questionable (its definitely not intended). But I don't agree that allowing NaN as valid value for float is the problem.

The problem IMO is more with the whole process of parsing and validating string values taken from XMLViews. While reviewing 544ee13, we discussed more complex changes that introduced new validation methods in DataType. At the end we decided against such a solution as it would have required more effort than expected and we assumed misspelled enums to be the most likely issue in practice.

The use of the native parseInt and parseFloat for DataType was appealing at the beginning of UI5. But for a qualified error reporting, those methods have the drawback that no feedback is given whether the input is fully consumed or not. The alternative would be to implement an own parsing, e.g. using regular expressions.

So the question to me is, whether fixing the discrepancy between int and float is what you aim for (I would then vote for allowing NaN for int) or is it rather a more complete solution for #2166 ?

[boghyon]

So the question to me is, whether fixing the discrepancy between int and float is what you aim for (I would then vote for allowing NaN for int) or is it rather a more complete solution for #2166?

That was also my conclusive question that I asked myself. Ideally, it's both. But if I had to chose one, I'd favor a complete solution for #2166 to improve developer experience / error reporting.

It's perfectly valid that NaN is of type number but having a typo in property value is something that should be notified about.

The problem IMO is more with the whole process of parsing and validating string values taken from XMLViews.

I think so too.

The alternative would be to implement an own parsing, e.g. using regular expressions.

👍

[stephania87]

I am logging this via 1880615058 for traceability and further internal discussions
BR

[codeworrior]

Note that commit 5aec8e6 was only meant to fix the inconsistency between the int and float types. It does not address the general topic of a better reporting of validation errors in XMLViews.

This has to be handled separately (kind of development request). I'll ask our PO how we want to track this (new internal incident, back log item or via #2166).