Number comparison doesn't handle NaN

[henbr]

This function doesn't handle NaN https://github.com/promaster-sdk/property/blob/master/packages/property/src/utils/compare-utils.ts#L16

This will cause this code to throw even though it looks like it shouldn't

  const pvs1 = PropertyValueSet.setInteger("test", NaN, PropertyValueSet.Empty);
  const pvs2 = PropertyValueSet.setInteger("test", NaN, PropertyValueSet.Empty);
  if (!PropertyValueSet.equals(pvs1, pvs2)) {
    throw Error("pvs not equal");
  }

[jonaskello]

@Jontem @AdamLuotonen Do you have an idea of how you would want this to work?

[AdamLuotonen]

I guess we should consider Infinity too in this discussion. Also, I think it should work the same way in Amount. Right now it is not clear what happens when you create an Amount of NaN or Infinity.

My first thought is that we should only allow values passing isFinite() in PVS, PF and Amounts. In general I think these values only arise due to some bug or missing data, and in those cases they are better represented by being absent in the PVS and checked using x=null in PF.

So unless there is some case where we need this distinction I think we should just throw an exception. Returning undefined from setInteger would probably not be a good user experience.

An alternative would be to not set the value at all and just return the original PVS. Maybe that would be too "quiet", but we could always log it to the console. This would help developers find these NaN/Infinity values and fix them. Any such values that slip through to production does not crash the whole program, they are just absent from the PVS. I think we already put some error to the log when we have invalid property filter syntax.