samuelgoto / proposal-number-parse

3 stars 0 forks source link

Concerns about deliberate backwards incompatibility #3

Open not-an-aardvark opened 7 years ago

not-an-aardvark commented 7 years ago

Based on https://github.com/tc39/proposal-numeric-separator/issues/11#issuecomment-314531138, it seems like this proposal will introduce deliberate backwards incompatibilities if the number literal syntax is enhanced in the future. For example, the value of Number.parse('1_0') will change depending on if the environment supports underscore separators (it will either be '1' or '10').

I'm confused because I don't see how this would be feasible. Based on https://github.com/tc39/ecma262/issues/927, parseInt and parseFloat can't be changed without breaking the web. If people start using Number.parse as a replacement for parseFloat, what will prevent Number.parse from ending up with the same problem?

ljharb commented 7 years ago

If Number can be changed, so can anything else.

It's changed with binary and octal literals in ES6 and not been a problem.

parseFloat and parseInt can't be changed because people coded based on no breaking changes brunt introduced - but that guarantee would never exist for this function.

not-an-aardvark commented 7 years ago

If Number can be changed, so can anything else.

Number produces NaN when it doesn't recognize something, whereas parseFloat produces a valid numerical result. This means that changes to Number take the form of a number replacing NaN, and changes to parseFloat result in a number replacing another number. In order to be affected by a breaking change to Number, a site would have to be relying on NaN being produced somewhere, and I suspect this is far less common than relying on a particular numerical value being produced. So I think there's reason to believe that parseFloat and Number.parse are more susceptible to breaking the web than Number().

parseFloat and parseInt can't be changed because people coded based on no breaking changes brunt introduced - but that guarantee would never exist for this function.

What are developers expected to do to account for this? It seems like it would be difficult to use Number.parse at all if it carried no guarantees about future breaking changes.

For example, if someone uses the example from the readme:

const value = Number.parse('1px');

// do something that relies on `value` being 1

This code will break in the event that number literal syntax is extended to add a p suffix, because Number.parse('1px') will return 1p instead of 1.

ljharb commented 7 years ago

In this case, this function would produce a string that would then be passed into Number, and used for no other purpose.

not-an-aardvark commented 7 years ago

Not sure I'm understanding -- how would passing the string into Number help with the backwards compatibility issue? It still seems like anyone using it would be at risk of having their code break later.

const str = Number.parse('1_0'); // '1' currently, '1_0' with future underscore separators
const value = Number(str); // Number('1') -> 1 currently, Number('1_0') -> 10 with separators

(By the way, is the function supposed to return strings? The readme says Number.parse("1px") === 1, but https://github.com/tc39/proposal-numeric-separator/issues/11 seemed to imply it would return a string.)

ljharb commented 7 years ago

Sorry, to clarify:

Number('0b1') was changed from returning NaN to 1. This function would have the contract "it returns a string that can be passed into Number()". Thus, it only has a breaking change when Number() does - which has been previously established as being acceptable.

not-an-aardvark commented 7 years ago

To me, it seems like the idea that Number changes are acceptable is convoluting the issue. Number changes have been acceptable in the past, but only because people don't typically rely on NaN. Number changes might not always be acceptable if another builtin uses Number as a dependency and has a more noticeable behavior switch based on what Number does.

I think it would be more accurate to frame the issue as:

Number changes have been acceptable in the past due to the former point, and parseFloat changes have been unacceptable due to the latter. If Number.parse is added, it will effectively violate the second point , regardless of whether the breaking change can actually be attributed to Number.

ljharb commented 7 years ago

Fair. That's an argument for Number.parse to do the Number() coercion itself, instead of returning a string.

not-an-aardvark commented 7 years ago

Agreed -- although to be clear, that won't solve the backwards compatibility issue. It will just make the API a bit easier to use.

ljharb commented 7 years ago

No, but it makes the backwards compatibility issue identical to Number() itself, which we've already established is fine to change.

not-an-aardvark commented 7 years ago

No, it doesn't -- Number.parse returns a number when it finds unrecognized characters, whereas Number returns NaN. This makes the backwards compatibility issue identical to parseFloat (and in category 2 from https://github.com/samuelgoto/proposal-number-parse/issues/3#issuecomment-315514737), which would make it unacceptable to change.

ljharb commented 7 years ago

The difference is that paraeFloat was used when there was an expectation it's rules would never change.

This method will always have the expectation that it's rules can change, so therefore any changes within its contract will always be acceptable.

not-an-aardvark commented 7 years ago

How/when are developers expected to use this function, if there's no guarantee that the rules are stable? To repeat the second part of https://github.com/samuelgoto/proposal-number-parse/issues/3#issuecomment-315498992, it seems like even idiomatic usage of this function would result in code that is at risk of breaking in the future.

I'm anticipating that if this function is added, one of the following will happen:

  1. Developers will rely on the output despite the weak contract, and it won't be possible to make improvements to the function without breaking the web.
  2. Developers will find the contract too weak to be useful in practice, because the function's behavior is basically undefined in the long term.
ljharb commented 7 years ago

The guarantee is "the return value from this function can be passed into Number()" (or, with the modification discussed, that it's a number).

If developers find the contract to be too weak to be useful in practice, then we haven't lost anything. If developers rely on the contract too strongly, then it's a failed experiment, and we won't be able to iterate as I expect.

However, there remains another possibility: that developers will only use it when they want the weak contract it provides, and it will be useful to extract numeric literals from the start of a string.

not-an-aardvark commented 7 years ago

Could you give an example of a use case that would only rely on the weak contract? It seems like extracting numeric literals from the start of a string would go beyond the contract (assuming the application actually cares what the resulting value is), because the function's treatment of the characters following the literal is undefined.

ljharb commented 7 years ago

It's not undefined, it's just defined to match what Number expects.

not-an-aardvark commented 7 years ago

Sorry -- to clarify, by "undefined behavior" I mean "might return a different value in the future if number literal syntax is enhanced".

ljharb commented 7 years ago

That's not undefined tho - that's strictly defined; it just potentially changes over time - just like Number() itself. If that's acceptable, so is this - if this is not, Number() changing isn't.

not-an-aardvark commented 7 years ago

Fair enough, calling the behavior "undefined" was misleading. I'll call it "future-unsafe behavior" from now on.

I'm still having trouble understanding a use case for this that wouldn't rely on future-unsafe behavior, though. (See https://github.com/samuelgoto/proposal-number-parse/issues/3#issuecomment-315667818, except swapping out the word "undefined" for "future-unsafe".)

ljharb commented 7 years ago

Every use case that calls for parseFloat.

not-an-aardvark commented 7 years ago

Using Number.parse as a replacement for parseFloat would result in fragile code that would break the web if the number literal syntax is enhanced (see the second part of https://github.com/samuelgoto/proposal-number-parse/issues/3#issuecomment-315498992).


It feels like this discussion is going in circles, which might be because I'm not communicating clearly. Let me summarize my argument:

  1. Sites rely on parseFloat returning a particular value, and they will break if it starts returning a different value for a given argument. In other words, sites are using parseFloat in a way that relies on it having a strong contract.
  2. If these sites start using Number.parse in place of parseFloat, they will be relying on Number.parse having a strong contract. As a result, upgrading Number.parse for future syntax extensions will not be web-compatible.
  3. You've mentioned that this wouldn't be a problem because Number.parse has a weak contract. Presumably, this means it would not be appropriate to use Number.parse in situations which require a strong contract.
  4. I think every use of parseFloat implicitly requires a strong contract*. (I'm not sure about this, but no counterexample has been presented yet.)
  5. Therefore, any usage of Number.parse as a replacement for parseFloat would be relying on a strong contract. Since Number.parse only provides a weak contract, upgrades to the number literal syntax could cause those sites to break, so they would not be web-compatible.

    * Except when the argument is a complete number literal with nothing else, but in those cases it's better to use Number anyway.

If you disagree with this, could you clarify which point you disagree with? We seem to agree on at least some parts of this, but I'm not sure which part you're objecting to.

ljharb commented 7 years ago

I disagree with point 4 (and thus 5). I agree with the first three points.

not-an-aardvark commented 7 years ago

Thanks for clarifying. Do you have a counterexample to point 4 (a use of parseFloat which doesn't require a strong contract)?

As a motivating example for why I think point 4 is correct: if a site relies on parseFloat('1px') returning 1, and p is added later as a suffix that changes the semantics of number literals, then the site will break if parseFloat starts supporting the suffix. This means that expecting parseFloat('1px') to equal 1 is relying on parseFloat's strong contract.

fuchsia commented 7 years ago

Number.parse() probably wouldn't handle suffixes. (I take it, it won't handle BigInts? So "6n" parses as 6? And, likewise, I assume "12in" would not rotate by 90 degrees if we added "i"-suffixed complex numbers?)

But how about if we extended numbers to support non-decimal fractions, like 0x0.4 === 0.25? (Hex floats allow you to enter exact bit patterns.) That would mean "0x0.ffff_ffff_ffff_ff" would change from being 0 to 1.

I'd happily add hex floats to Number, because I don't let Number() near unsanitised user input. However, I would expect a function called parse to be safe on user-supplied strings. If it's meaning might change in the future, I'd have to stick with parsing strings manually and handing them off to Number() as I do at present.

ljharb commented 7 years ago

Maybe extractLiteral is a better name?

fuchsia commented 7 years ago

Yeah, if you call something a bikeshed people expect to be able to store their bikes in it. The version I've just prototyped I called Number.tokenize(). (My version takes a string and an optional startIndex :P and returns the longest string, including trailing whitespace, that could be coerced via Number. I'm going to see if it's actually useful.)

extractLiteral() is a bit verbose, but it would shape people's expectations towards the function you seem to want, and I would get behind it over Number.parse().