isString and TypeScript narrowing about radash

[matteosacchetto]

@rayepps did a PR for this

[matteosacchetto]

It should be as easy as this:

export const isString = (value: any): value is string => {
    return typeof value === 'string' || value instanceof String
}

Simply add a TS guard as return type

Reference: https://www.typescriptlang.org/docs/handbook/2/narrowing.html#using-type-predicates

[matteosacchetto]

I was thinking about how to implement this.
I wanted to do a PR addressing all the different is* functions, but I have a question, so wanted to discuss here a bit before addressing it.

Given that the different tests do a check with typeof '<primite_type>' || instanceof <Class>, what should be the type of the variable?

In the example above should it be

export const isString = (value: any): value is string => {
    return typeof value === 'string' || value instanceof String
}

or

export const isString = (value: any): value is string | String => {
    return typeof value === 'string' || value instanceof String
}

The latter one is more correct, but may be more "painful" to work with, since if you do so then value becomes value: string | String so functions accepting string may complain

Since the case of using the String class are practically non-existing, it may be safer to do the following

export const isString = (value: any): value is string => {
    return typeof value === 'string'
}

and then expose a method isClass to check if is instance of class, what are your thoughts on this?

[sodiray]

Wow, I have never seen this type predicate before, I didn't know this existed. I'd love to add this to all the isN functions in the typed module.

@matteosacchetto regarding your question, it seems valid both ways but I'd probably prefer value is string. For 99% of people I think this will be the happy path. For the 1% who need the value to be a String instance type they can cast it.

[matteosacchetto]

So, just to be on the same page, this is the solution you prefer, right?

export const isString = (value: any): value is string => {
    return typeof value === 'string' || value instanceof String
}

If so I will proceed and do a PR and add type guards to all isN methods

[sodiray]

Yes 👍 it would be awesome if you made a PR but I can also do it. I'd like to get it in for the 8.0.0 release with the ESM change (today or tomorrow)

[matteosacchetto]

I have a question on the isN functions:

export const isSymbol = (value: any): value is symbol => {
  return !!value && value.constructor === Symbol
}

Why in some functions you didn't use typeof or instanceof like for isString?

[sodiray]

For each is check I used the most dependable check I could find on google, stackoverflow, and other libraries. Because Javascript is a quirky language using instanceof is not always the most reliable way to check the type of a variable at runtime.

A good example is isDate, as mentioned in this SO post using instanceof on a Date will fail when passing across frame boundaries.

There are lots of little quirks like that we want to account for.

[matteosacchetto]

Ok, good to know, thank you for the details 😉

[charleskoehl]

Thanks guys I didn't get notifications of this activity or I would have responded sooner.