Skip to content

feat: refine .split() return type #56834

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
wants to merge 1 commit into from

Conversation

castarco
Copy link

Work in Progress:

  • Searching if there is an appropriate related issue
    • If not, prepare a properly described issue to describe the problem
  • Prepare new specific tests

Signed-off-by: Andres Correa Casablanca <castarco@coderspirit.xyz>
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Dec 20, 2023
@castarco
Copy link
Author

@microsoft-github-policy-service agree

@fatcerberus
Copy link

Searching if there is an appropriate related issue

I found two:

@MartinJohns
Copy link
Contributor

Your changed type definitions fail with "".split("").

@castarco
Copy link
Author

castarco commented Dec 21, 2023

@MartinJohns

Your changed type definitions fail with "".split("").

NOTE: I edited this post to correct an incorrect statement I made a few minutes ago.
NOTE 2: As I wrote in an addendum at the end of this message, I have hope we can improve the return type (in a smaller set of cases than I originally thought possible, though) by following the "opposite" approach I took in this PR.

Yes, I noticed some time later 😞 . I was thinking if it was possible to cover those corner cases with some extra function overrides... but that won't be possible (for now), because there's a more generic case quite difficult to cover:

const strValue = ''
const a = strValue
const b = strValue

function myUselessFunction(x: string, y: string): string[] {
  // Because we don't know the values of `x` and `y`, we can't produce
  // any reliable inference on the result's type :( .
  return x.split(y)
}

const c = myUselessFunction(a, b)

So I guess I'll have to close this one.

As a side note, this problem (and other similar ones) could be "mitigated" (but not completely) with any or many of the following features:

  1. Some feature to ensure that any two function params are not aliases of each other... but that's a complex topic (I guess it's only possible in a very narrow set of cases).
  2. A more powerful version of the Exclude utility type allowing us to create narrowed-down versions of "basic" types (not just for unions).
  3. A mechanism (just internal, I guess) to refine interfaces for specific literals. That is, given the interface for the string type, being able to have more specific versions of that interface for particular values (or narrowed-down types) of string (or any other "basic" type, like number)... so we could have a specialised interface for '', or 0.

Regarding the second option I listed here, we can already do something similar enough with conditional types... but they only work well when used in function params position and its usefulness is very limited:

type NonEmptyString<S extends string = string> = S extends '' ? never : S

// The return type won't be enough for TS to know that the returned value is
// not '' once it is used elsewhere. So this way to discriminate against '' is
// only valid at the call site.
function g<S extends string>(s: NonEmptyString<S>): NonEmptyString<S> {
  if (s === '') {
    throw new Error('simplest way to avoid having to rely on a union return type')
  }
  return s as NonEmptyString<S>
}

// The type system will complain, but... only for statically-known literals
const vvv = g('')

P.S.:

We could apply the opposite approach I followed in this PR: instead of trying to consider the cases where the return type is string[] as special cases, and [string, ...string[]] as the general case, we can try treating the "[string, ...string[]] return type" case as the special case. 🤔

So, if we statically know that our separator is NOT '', and ALSO that the second parameter is NOT 0, we can change the return type to [string, ...string[]]. But I'll do that in a separate PR.

@castarco castarco closed this Dec 21, 2023
@castarco
Copy link
Author

My new attempt: #56841 .

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.

4 participants