microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
99.07k stars 12.29k forks source link

Do not allow surrounding spaces to be counted as part of ${number} templates in template index strings #46109

Open craigphicks opened 2 years ago

craigphicks commented 2 years ago

Suggestion

πŸ” Search Terms

Template String Pattern Index Signatures Template Index Strings

βœ… Viability Checklist

My suggestion meets these guidelines:

Actually it would change the typescript behavior as it was released - maybe better now than later? The current behavior might be more bug than feature.

⭐ Suggestion

Not allowing spaces would reduce typing mistakes.

πŸ“ƒ Motivating Example

Modification of the example feature explanation in the handbook:

interface Options {
    width?: number;
    height?: number;
}

let a: Options = {
    width: 100,
    height: 100,
    "data-1": true, // Error! 'data-blah' wasn't declared in 'Options'.
};

interface OptionsWithDataProps extends Options {
    // Permit any property matching 'data-${number}'.
    [optName: `data-${number}`]: unknown;
}

let b: OptionsWithDataProps = {
    width: 100,
    height: 100,
    "data-1": true,       // Works!
    "data- 1": true,       // Passes but might be better off failing
    "data-1 ": true,       // Passes but might be better off failing
    "data- 1 ": true,       // Passes but might be better off failing
    "data- ": true,       // Passes but might be better off failing  (no number at all)

    "unknown-property": true,  // Error! 'unknown-property' wasn't declared in 'OptionsWithDataProps'.
};

playground

πŸ’» Use Cases

Using it to catch unintended key entries.

IllusionMH commented 2 years ago

Can be simplified to

const a: `data-${number}` = "data-1"; // OK

// No errors
const a1: `data-${number}` = "data- 1";
const a2: `data-${number}` = "data-1 ";
const a3: `data-${number}` = "data- 1 ";
const a4: `data-${number}` = "data- ";

Playground

jcalz commented 2 years ago

Oh wow this an even better example for my list of surprising behaviors of `${number}` in #41893! I'm still not quite able to internalize the interpretation of `${number}` meaning "some string that converts to a finite number", but apparently that's what it means (as opposed to "the type of `${n}` when n is a finite number").

craigphicks commented 2 years ago

@jcalz - Thanks for link to 7.1.4.1.2 Runtime Semantics: StringNumericValue, that is very informative. So the behavior is intended to match some formal standard - as I suspected but wasn't sure. (I also enjoyed you list of examples!)

For a user on the consuming end of the data, if they want to correctly decode the pattern suffix to disambiguate the many-to-one mapping of strings to numbers, they must use of algorithm listed in [7.1.4.1.2 Runtime Semantics: StringNumericValue]. I see that a possible source of confusion and errors. That risk would be mitigated by employing a simple one-to-one mapping.

Having a many-to-one mapping of keys strings to semantic keys, before using the keys to lookup the value in the object map seems anti intuitive and error prone. But the original problem lies in JS, not TS.

On the other hand, if TS is going to impose some strict typing on the keys for arbitrary integers, why not go the whole way and allow specification of a one-to-one map? For an explicit limited range of integers it is already possible:

type CountingIntegers = "1"|"2";
type FooN = Record<`foo${CountingIntegers}`,any>

Maybe the feature request should be for a temple-key-string use specific intrinsic utility type "CountingIntegers" allowing all nonnegative integers up to Number.MAX_SAFE_INTEGER (or maybe convertible to bigint). That is narrow enough to be safe and easy to implement, is extremely simple for a user to understand completely at a glance, and has practical value in eliminating misunderstandings.

craigphicks commented 2 years ago

Thanks to @jcalz pointing out 7.1.4.1.2 Runtime Semantics: StringNumericValue, it is now clear within a shadow of a doubt that the "surrounding space" behavior is "working as intended". So I will close this issue.

jcalz commented 2 years ago

I’m not sure… I’d really like to hear someone officially say that this is intentional. I’m just an interested bystander.

IllusionMH commented 2 years ago

Same here regarding whitespace. While 100.1 or even 1e3 are really looking as number literals including whitespace doesn't look expected from usage perspective, but that's IMHO.

andrewbranch commented 2 years ago

This is very surprising to me and seems undesirable, but I’ll let @ahejlsberg have the definitive word.

ahejlsberg commented 2 years ago

Yeah, we're basically just relying on JavaScript's intrinsic string-to-number conversion to validate `${number}` placeholder values--and it permits leading and trailing whitespace. Also, strangely, it permits empty strings and strings with only whitespace and returns 0 for those.

I agree it makes more sense to not permit any whitespace and in particular to not permit empty strings. It's technically a breaking change, but can't imagine many types depend on it.