Skip to content

Support string literals as spread argument #51397

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
5 tasks done
philer opened this issue Nov 3, 2022 · 9 comments
Closed
5 tasks done

Support string literals as spread argument #51397

philer opened this issue Nov 3, 2022 · 9 comments
Labels
Duplicate An existing issue was already created

Comments

@philer
Copy link

philer commented Nov 3, 2022

Suggestion

A tuple of characters can be concisely represented as a string literal – for example "0123456789" is a convenient representation of ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"] for many purposes. However when using the spread operator ... on a string literal, e.g. in a function call, TS2556 is raised with the message: "A spread argument must either have a tuple type or be passed to a rest parameter".

My suggestion is therefor to allow string literals to be treated like string tuples in conjunction with the spread operator.

🔍 Search Terms

string literal, spread operator, spread string literal, spread characters, string as tuple

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

When using the spread operator on a string literal it should work the same way as when it's used on a string tuple. E.g. the following expressions are equivalent in plain JS and thus should be supported the same way in TS:

fn("a", "b", "c")  // OK
fn(...["a", "b", "c"] as const)  // OK but impractical
fn(..."abc")  // Fails but should be OK

📃 Motivating Example

I'm using ts-pattern to match keyboard events. For this example let's say I want to use it to control volume:

const handleKeyDown = (evt: KeyboardEvent) =>
  match(evt.key)
      .with("+", () => updateVolume(vol => vol + 0.1))
      .with("-", () => updateVolume(vol => vol - 0.1))
      .with(..."0123456789", digit => updateVolume(0.1 * digit))
      .otherwise(noOp)

For the +/- keys, this would increment/decrement the volume by 10%. When pressing a number key it should set the volume directly to the corresponding multiple of 10%.
Note that ts-pattern's .with method takes any number of arguments for pattern matching and if any of the arguments matches the input calls the final argument as a function. Of the example above the fifth line does not work with current TS (4.8.4). I've tried the following variations, all of which fail type checking:

      .with(..."0123456789", digit => updateVolume(0.1 * digit))
      .with(...("0123456789" as const), digit => updateVolume(0.1 * digit))
      .with(...([..."0123456789"] as const), digit => updateVolume(0.1 * digit))
      .with(...(Array.from("0123456789" as const)), digit => updateVolume(0.1 * digit))

However the following work:

      .with("0", "1", "2", "3", "4", "5", "6", "7", "8", "9", digit => updateVolume(0.1 * digit))
      .with(...["0", "1", "2", "3", "4", "5", "6", "7", "8", "9"] as const, digit => updateVolume(0.1 * digit))

You might call this a matter of convenience but I would argue that it improves readability in those cases where it comes in handy – and code readability could always be argued to be "just" a convenience.

💻 Use Cases

Whenever you want to hard code a tuple of characters it is easy to either use a string literal directly or spread it into an array, such as

const alphabet = [..."abcdefghijklmnopqrstuvwyz"] as const

This is quite universal and can be useful whenever you need a hard coded tuple of characters. Some examples:

  • all digits, all latin letters, etc. (seen above)
    • when handling user input
    • building a parser of some sort
  • all characters valid in a password
  • "wasd" (video game keyboard controls)
  • Characters are sometimes used like flags (e.g. as the second argument to the RegExp constructor).
  • Many educational examples/exercises revolve around string manipulation (e.g. on codewars.com).
@MartinJohns
Copy link
Contributor

MartinJohns commented Nov 3, 2022

This is intentional. Duplicate of #39292 (comment).

As mentioned in the other issue, intentionally wanting to spread a string is very rare. Personally I had it much more often that I accidentally tried to spread a union type that might be a string and the compiler protected me from this mistake.

@RyanCavanaugh
Copy link
Member

String spread is fine for ASCII stuff, but quickly goes off the rails for more complex characters. e.g. [..."🤷‍♂️👴🏻"] is ['🤷', '‍', '♂', '️', '👴', '🏻'], [...", ֶ, ִ, ֹ"] turns into [',', ' ', 'ֶ', ',', ' ', 'ִ', ',', ' ', 'ֹ'].

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Nov 3, 2022
@philer
Copy link
Author

philer commented Nov 3, 2022

Well, that's a shame. Calling this feature "very rare" seems rather subjective, if not speculative.

I was under the impression that TS tries to add type safety to JS, not remove features that might backfire when used carelessly. With that reasoning you might as well forbid half of JS's standard library.

@RyanCavanaugh Emojis and other weird utf characters are always trouble when manipulating strings, this is not exclusive to spread syntax. You can shoot yourself in the same foot with pretty much any String method, or any other use case where strings are treated as Iterables (which TS does support!). And putting invisible whitespace in a string literal is pretty much exclusively useful for trolling coworkers…

@RyanCavanaugh
Copy link
Member

Every type error we issue is a subjective call of what is and isn't a good idea in JS. There's no bright line anywhere.

@fatcerberus
Copy link

fatcerberus commented Nov 4, 2022

I was under the impression that TS tries to add type safety to JS, not remove features that might backfire when used carelessly.

Except that’s exactly what TS does, almost by definition. There is no undefined behavior in JS, but a lot of unintuitive behavior. See, e.g. #38471

@philer
Copy link
Author

philer commented Nov 5, 2022

@fatcerberus parseInt is basically runtime type casting, so naturally it's prone to some special treatment when it comes to static type checking. In fact, parseInt is exceptionally treacherous and deserves exceptional treatment – but IMHO that should still remain an exception. Using it as a precedent for removing features with pitfalls is an insidious argument, as it can be applied to virtually any feature.
Plus you can still (ab)use parseInt by adding a type assertion, so you can still access that functionality if you really want to. String literal spreading on the other hand is just basic syntax sugar for a feature that TS already otherwise supports (iterating strings). But in its current state there is no practical way of forcing the compiler to accept it.


Anyway, I'm going to stop trying to argue my point here. I'm a bit disappointed with the offhanded dismissal of my proposal – it does not feel like there was any real attempt at objective consideration. Though of course I don't know all of TS's history and it is very possible that I'm biased.

@fatcerberus
Copy link

fatcerberus commented Nov 5, 2022

Using it as a precedent for removing features with pitfalls is an insidious argument, as it can be applied to virtually any feature.

The relevant question is "if this feature is allowed by the type system, will it always do what most people expect it to?" In the case of parseInt(numVal), people tend to read this as "get the integer part of numVal"--which of course it doesn't do if the number is too large--it returns garbage instead--hence making it dangerous. In the case of [ ...stringVal ], most people IMO will read this as "split stringVal into characters" (indeed, your exact use case) - which works for ASCII strings but breaks down for Unicode in general, because it actually means "split into UTF-16 codepoints". This difference in expected vs. actual behavior is likely to be surprising, and, apparently from TS's point of view, dangerous. This is all completely subjective, of course, but the entire type system has been built on these kinds of subjective judgment calls, for better or worse.

At any rate, you can still cast the string literal (..."abc" as any as string[]).

But in its current state there is no practical way of forcing the compiler to accept it.

Everything above having been said, what you're trying to do does, in fact, appear to work already:

function fn(...args: string[]) { console.log(args); }
fn(..."abc");  // no error - prints a, b, c
const alphabet = [ ..."abcdefghijklmnopqrstuvwxyz" ];  // also no error
console.log(alphabet);  // prints alphabet split by character

TypeScript Playground link

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Nov 7, 2022

Anyway, I'm going to stop trying to argue my point here. I'm a bit disappointed with the offhanded dismissal of my proposal – it does not feel like there was any real attempt at objective consideration.

We obviously put a lot of consideration into how this feature works when implementing it; re-enacting that design conversation with every user who disagrees is not scalable. We told you why the feature works the way it does, and that it was on purpose. You're welcome to offer a rebuttal and try to change our minds, but this is fundamentally a bug tracker, and if your suggestion boils down to "Your design outcome should have gone the other way" without raising novel points, there's not much else for me to say.

@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

5 participants