radashi-org / radashi

The modern, community-first TypeScript toolkit with all of the fast, readable, and minimal utility functions you need. Type-safe, dependency-free, tree-shakeable, fully tested.
https://radashi.js.org
MIT License
315 stars 25 forks source link

Enhance `isObject` type Inference for `function` and `object` parameters #257

Open MarlonPassos-git opened 2 months ago

MarlonPassos-git commented 2 months ago

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](https://github.com/user-attachments/assets/355b8e60-a82c-40f4-b741-a44b0810f6b8) ![image](https://github.com/user-attachments/assets/e7d3a9f8-ceb0-44b8-baa9-6afa4145bbd6) ![image](https://github.com/user-attachments/assets/06fd1ae5-2c7b-4b27-94f7-3d0f73a7b9bf)

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](https://github.com/user-attachments/assets/b80515b8-a2b1-421b-aead-dd86529b05d5) ![image](https://github.com/user-attachments/assets/2ca06c66-c3bd-4963-997b-5bf1e90c8bb3)

when fixing this create the type tests for the function.

aleclarson commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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