Deserializing empty attributes when the property type definition is Number results in "0"

[klebba]

Is it a bug?

Consider:

<x-foo baz></x-foo>

Where baz is defined as a Number type property; the value of baz will be coerced to Number 0. This is because, in Javascript, Number('') === 0.

[klebba]

Perhaps:

else if (property.type === Number && value === '') {
  // Handle the case where Number('') === 0
  return Number.NaN;
}

https://github.com/Netflix/x-element/blob/main/x-element.js#L843

One reason not to do this is that it's not actually the normal behavior of JavaScript, because Number('') does become the Number 0 — HOWEVER an empty attribute value needing to be deserialized is a special case (in other words the empty string originates from the DOM API element.getAttribute('baz') === '' — so it seems we have the need to make a decision in x-element (there is no obvious standard to follow)

[klebba]

Funny:

Number('                 ')

This is zero. Wat.

Update: we could do:

'         '.trim() === ''

[theengineear]

Thanks for the discussion on this one yesterday, @klebba. I think '' >> NaN makes sense for Number. However, it’s interesting to note that the same would not work if BigInt was supported as a serializable type. As far as I understand it, there is no concept of NaN or Infinity when dealing with BigInt. Consider the following cases:

// Coercion of empty string seems roughly equivalent.
Number('') // 0
BigInt('') // 0n

// Where Numbers will quietly convert to NaN, BigInt will just throw.
Number('foobar') // NaN
BigInt('foobar') // Uncaught SyntaxError: Cannot convert foobar to a BigInt

// Where Numbers accommodate Infinity, BigInt will just throw.
Number('Infinity') // Infinity
BigInt('Infinity') // Uncaught SyntaxError: Cannot convert Infinity to a BigInt

// Where Number arithmetic can result in Infinity...
zeroNumber = Number('0')
oneNumber = Number('1')
oneNumber / zeroNumber // Infinity

// ... BigInt arithmetic will just throw.
zeroBigInt = BigInt('0')
oneBigInt = BigInt('1')
oneBigInt / zeroBigInt // Uncaught RangeError: Division by zero

Ultimately, I think if / when we would decide to allow BigInt to be deserialized. We would want to throw an error. I.e., <my-element big-int=""> and <my-element big-int> would throw a halting error (similar to how setting an incorrectly typed value on a property throws a halting error).

[klebba]

Agree about BigInt — I'm not as familiar with this type, but prefer to YAGNI its handling for now