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

feat: refine .split() return type #56841

Closed
wants to merge 1 commit into from

Conversation

castarco
Copy link

@castarco castarco commented Dec 21, 2023

NOTE: ⚠️ Still work in progress. ⚠️

PR Description

This PR refines String.split(*) return type (plus the RegExp's cousin method).

As opposed to previous attempts, it does not try to characterise the "empty array result" as a special case, but quite the opposite, it characterises "[string, ...string[]] result" as a special case.

With this change, we only get [string, ...string[]] as return type when we can statically know for sure that we are not trying to execute ''.split(''), and we do not set the second parameter (limit) to 0.

In other words, the separator must be a literal for this to give as a "nice" return type. If we use a variable, we'll keep the more general string[] return type.

This (combined with the fact that we can't specialise interfaces for specific literals) means that there are cases where we still get non-empty arrays as a result but our return type won't be able to represent that fact.

Other Relevant Details

I introduced the types NonEmptyStringParam<> and NonZeroNumberParam<>. I suspect that these 2 new types might be not well received. I'm not sure how to make the non-global.

Pending tasks:

  • Fix formatting issues
  • Fix baseline tests
  • Add new tests

What this PR tries to fix

This change still requires to fix the tests.

Signed-off-by: Andres Correa Casablanca <[email protected]>
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 21, 2023
@castarco castarco mentioned this pull request Dec 21, 2023
3 tasks
@MartinJohns
Copy link
Contributor

MartinJohns commented Dec 21, 2023

No idea why you decided to open another PR instead of just changing your existing. Your PR is still about the same issue, and splitting it like this just scatters the discussion.

Nothing proposed here hasn't been proposed before. See also: #53362 (comment)

we have a pretty low appetite for making the standard library definitions extremely complex for the sake of NUIA

I'd argue your suggested change is very complex. See also: #56333 (comment)

@castarco
Copy link
Author

@MartinJohns Ok. 🤷🏻 . I guess there's no point on fixing the tests then. (and sorry for the duplicate PR).

@castarco castarco closed this Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve string split return type of first array index
3 participants