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

fromEntries should return partial type #225

Open
IlyaSemenov opened this issue Sep 1, 2023 · 5 comments
Open

fromEntries should return partial type #225

IlyaSemenov opened this issue Sep 1, 2023 · 5 comments

Comments

@IlyaSemenov
Copy link

Bug Report

💻 Code

type Obj = {
  type: "foo" | "bar"
  size: number
}

const objs: Obj[] = []

const res = fromEntries(objs.map((obj) => [obj.type, obj]))
console.log(res.foo.size)

🙁 Actual behavior

There is no typing error. The code crashes.

🙂 Expected behavior

Typing error: 'res.foo' is possibly 'undefined'.

Version Information

  • Operating system: macOS 13.14.1
  • Typescript: 5.2.2
  • Nodejs: 20.4.0
@shine1594
Copy link
Collaborator

This issue is about how sound TypeScript's type system is. This level of soundness is common in TypeScript, much like a case where you access an element in an array:

const arr = [1,2,3];
arr[100] // number;

According to TypeScript's design goals, there is a emphasis on balancing productivity with complete type soundness. Therefore, I think keeping fromEntries as it is aligns with TypeScript's manner.

@IlyaSemenov
Copy link
Author

Sorry, but I'm not following. What's the benefit of emitting knowingly incorrect type in this particular case?

Basically, the user is forced to do this every time:

const res = fromEntries(...)
return res as Partial<typeof res>

What do we lose if Partial<...> is applied automatically by fromEntries? Does it somehow worsen the DX?

@shine1594
Copy link
Collaborator

Sorry, but I'm not following. What's the benefit of emitting knowingly incorrect type in this particular case?

Basically, the user is forced to do this every time:

const res = fromEntries(...)
return res as Partial<typeof res>

What do we lose if Partial<...> is applied automatically by fromEntries? Does it somehow worsen the DX?

If you do it as you suggest, conversely, you'd still need to perform a null check, even when the input to fromEntries is definitely not an empty array.

const res = fromEntries([['a', 1], ['b', 2]])
// typeof res : { a?: number, b?: number }

if (res.a !== undefined) {
  console.log(res.a)
}

When the input to fromEntries is an empty array, it's an unintended exception for the developer. I think it's better for DX to avoid unnecessary null checks in most valid scenarios. Even if it means sacrificing complete type soundness for rare edge cases. Especially if it aligns with TypeScript conventions.

@IlyaSemenov
Copy link
Author

I thought the whole point of fromPairs is to process dynamic pairs. For a static list of pairs, it seemingly gives no benefit: one could simply write { a: 1, b: 2 } in the first place. However, for dynamic lists of pairs, Partial<> starts to make more sense. Anyhow, I understand your reasoning, thank you for the response.

@shine1594
Copy link
Collaborator

shine1594 commented Sep 1, 2023

I understand your statement thoroughly, and I agree with your point. However, it ultimately boils down to a trade-off. The groupBy function below encounters a similar issue, and it's a problem that everyone inevitably faces when using TypeScript.

type Obj = { type: 'A' | 'B' | 'C' };
groupBy(
  ({ type }) => type,
  list as Obj[]
) // `{ A: Obj, B: Obj, C: Obj }` or `{ A?: Obj, B?: Obj, C?: Obj }` ?

I think that inferring it as { A: Obj, B: Obj, C: Obj } is the better choice. The same applies to the fromEntries.

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

No branches or pull requests

2 participants