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

Enhance isObject type Inference for function and object parameters #257

Open
MarlonPassos-git opened this issue Sep 27, 2024 · 5 comments
Open
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@MarlonPassos-git
Copy link
Contributor

MarlonPassos-git commented Sep 27, 2024

The types of the current code are wrong

const returnSomething: () => (() => void) | { a: string } = () => ({ a: 'a' })
const something = returnSomething()

if (isObject(something)) {
  something // const something: (() => void) | {a: string }
} else {
  something // const something: never
}
See evidences

image
image
image

my suggestion to solve this

export function isObject<T extends Record<string|number|symbol, unknown>, S>(value: T | S): value is T 
export function isObject(value: unknown): value is object {
  return isTagged(value, '[object Object]')
}
See results

image
image

when fixing this create the type tests for the function.

@MarlonPassos-git MarlonPassos-git added bug Something isn't working good first issue Good for newcomers labels Sep 27, 2024
@MarlonPassos-git MarlonPassos-git changed the title Improve isObject type infer when passing a parameter with the type is function or object Enhance isObject type Inference for function and object parameters Sep 27, 2024
@aleclarson
Copy link
Member

You can simplify the suggested fix to:

export function isObject(value: unknown): value is Record<keyof any, unknown> {
  return isTagged(value, '[object Object]')
}

…and you would get the same effect.

The only problem is that interface types are famously not assignable to Record types (except in some cases).

type Point = { x: number; y: number }

interface IDog {
  bark(): void
}

interface StringToStringMap {
  [key: string]: string
}

interface KeyToStringMap {
  [key: keyof any]: string
}

declare const x: Record<string, string> | KeyToStringMap | StringToStringMap | IDog | Point | (() => void) | Function

if (isObject(x)) {
  x // => Point | KeyToStringMap | Record<string, string>
}

In the example above, you would expect x to also include the StringToStringMap and IDog types.

@aleclarson
Copy link
Member

aleclarson commented Sep 27, 2024

I don't think isObject can be correctly typed with the current version of TypeScript, sadly 🥲

So I guess the question is, what's the least worst solution?

@aleclarson
Copy link
Member

aleclarson commented Sep 27, 2024

Here's my proposal:

interface ObjectLike {
  [key: string]: unknown
}

export function isObject<T = never>(value: unknown): value is T | ObjectLike {
  return isTagged(value, '[object Object]')
}

You can call it without a type parameter, and it will work for everything except interface types that don't have an index signature.

type Point = { x: number; y: number }

interface IDog {
  bark(): void
}

interface StringToStringMap {
  [key: string]: string
}

interface KeyToStringMap {
  [key: keyof any]: string
}

declare const x: Record<string, string> | KeyToStringMap | StringToStringMap | IDog | Point | (() => void) | Function | RegExp

if (isObject(x)) {
  x // => Point | StringToStringMap | KeyToStringMap | Record<string, string>
}

You can call it with a type parameter to ensure it works for such interface types as well. Of course, this is not ideal, since it can be a maintenance burden, but it's a decent enough stop-gap solution, in my opinion.

if (isObject<IDog>(x)) {
  x // => IDog | Point | StringToStringMap | KeyToStringMap | Record<string, string>
}

If x could possibly be another interface type with no index signature, you'd have to add it to the type parameter to ensure type narrowing works as expected.

if (isObject<IDog | ICat>(x)) {
  x // => IDog | ICat | Point | StringToStringMap | KeyToStringMap | Record<string, string>
}

@aleclarson
Copy link
Member

aleclarson commented Sep 27, 2024

Actually, I think we could get a complex conditional type to work without the need for explicit type parameters. It would need to rely on BuiltInType and some other checks.

@MarlonPassos-git
Copy link
Contributor Author

I didn't know about this limitation, good to know. And even better to know that there might be a solution, no matter how complex it is.

I like your suggestion, it would already be good enough for now, given this limitation of some types having to be passed as generics.

The only thing I would change is to make the generic extend from an object.

export function isObject<T extends object = never>(value: unknown): value is T | ObjectLike {
  return isTagged(value, '[object Object]')
}

To block a weird thing like this:

declare const x: Record<string, string> | KeyToStringMap | StringToStringMap | IDog | Point | (() => void) | Function | RegExp | string | number

if (isObject<number>(x)) {
  x // => number | Point | StringToStringMap | KeyToStringMap | Record<string, string>
} else {
  x // => string | number | Function | RegExp | IDog | (() => void)
}

It would still allow this isObject<()=> void>(x), but at least it avoids primitives

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants