Skip to content
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

improve types for String.split #110

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ivanhofer
Copy link

This PR improves the type definitions for String.split. By default it always returns string[] but depending on the arguments passed, we can infer a few cases e.g.

  • Splitting a string will almost always return an array with at least one item, so the type [string, ...string[]] makes more sense.
  • Unless when the string and separator is an empty string '', then an empty array [] is returned. In that case string[] makes sense again
  • When the limit is set to 0 an empty array [] gets returned.

@lo1tuma
Copy link

lo1tuma commented Sep 23, 2023

FWIW: there is also on open issue in the typescript repo, which hasn’t been closed yet. So there is a chance that this will be supported upstream.

That’s being said, I would be very happy about such an improved version of String.prototype.split. No matter if it is in typescript directly, or here via ts-reset.

@@ -0,0 +1,5 @@
interface String {
split<Separator extends string, Limit extends number>(separator: Separator, limit?: Limit): Limit extends 0 ? []
: Separator extends '' ? string[]
Copy link

@lo1tuma lo1tuma Sep 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be dangerous when the type is not inferred by a literal and an empty string is passed as type string. For example:

const foo: string = '';
''.split(foo); // <-- would incorrectly yield [string, ...string[]]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants