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

is.numericString type guarding returns x is string and messes up typescript #176

Closed
Xananax opened this issue Dec 7, 2022 · 4 comments · Fixed by #178
Closed

is.numericString type guarding returns x is string and messes up typescript #176

Xananax opened this issue Dec 7, 2022 · 4 comments · Fixed by #178

Comments

@Xananax
Copy link
Contributor

Xananax commented Dec 7, 2022

Use case:

const parseValue = (/**@type {string} */value) => {
  if (is.numericString(value)) {
    return parseFloat(value);
  }
  if (value.toLowerCase() === "true" || value.toLowerCase() === "false") {
    return value.toLowerCase() === "true";
  }
  //...
}

Typescript assumes value is a string and then casts value to never:
image

I'm not sure if this is solvable, because strictly speaking the type guard is accurate.

Workaround (for anyone who needs it): recast the value manually:

JS:

value = /**@type {string}*/(value)

TS:

value = value as string

Notably, this recasting is possible in an early return context like my example, but not in an if...else or switch context, so it is still a bit of a bother.

The recasting is also surprising and requires a comment for other teammates that may come across it.

@Xananax Xananax changed the title is.numericString type guarding returns x is string and messes up typescript is.numericString type guarding returns x is string and messes up typescript Dec 7, 2022
@sindresorhus
Copy link
Owner

Maybe we could make the type guard more specific using a technique like this: https://github.com/sindresorhus/type-fest/blob/0b78096186dfe255b888513d60538e17e35828ea/source/internal.d.ts#L108-L111

@Xananax
Copy link
Contributor Author

Xananax commented Dec 11, 2022

Oh that sounds pretty good and seems like an easy change. Want me to PR this?

@vtgn
Copy link

vtgn commented May 30, 2023

Use case:

const parseValue = (/**@type {string} */value) => {
  if (is.numericString(value)) {
    return parseFloat(value);
  }
  if (value.toLowerCase() === "true" || value.toLowerCase() === "false") {
    return value.toLowerCase() === "true";
  }
  //...
}

Typescript assumes value is a string and then casts value to never: image

I'm not sure if this is solvable, because strictly speaking the type guard is accurate.

Workaround (for anyone who needs it): recast the value manually:

JS:

value = /**@type {string}*/(value)

TS:

value = value as string

Notably, this recasting is possible in an early return context like my example, but not in an if...else or switch context, so it is still a bit of a bother.

The recasting is also surprising and requires a comment for other teammates that may come across it.

Hi!

The problem is that in a predicate function, the type guard must always be accurate: if the predicate on a given value returns true, the value must be of the given type in the type guard AND if the predicate returns false, the value must not be of the given type in the type guard.
And this is not the case in the is.numericString predicate. So the solution is indeed to modify the type guard to use a type representing all the numeric string values.

@Xananax
Copy link
Contributor Author

Xananax commented May 30, 2023

I confirm that changing the signature of numericString like so:

var numericString: (value: unknown) => value is `${number}`;

fixes the problem.

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 a pull request may close this issue.

3 participants