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

PickDeep strips null #880

Open
relsunkaev opened this issue May 15, 2024 · 7 comments
Open

PickDeep strips null #880

relsunkaev opened this issue May 15, 2024 · 7 comments
Assignees

Comments

@relsunkaev
Copy link

relsunkaev commented May 15, 2024

If you have following types

type Owner = {
  name: string;
  age: number;
};
type Pet = {
  name: string;
  age: number;
  owner: Owner | null;
};

the resulting type of PickDeep<Pet, "name" | "owner.name"> is

type Result = {
    name: string;
    // `owner` is no longer nullable
    owner: {
        name: string;
    };
}

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • The funding will be given to active contributors.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar
@Emiyaaaaa Emiyaaaaa self-assigned this May 16, 2024
@sindresorhus sindresorhus added the bug Something isn't working label May 16, 2024
@Emiyaaaaa
Copy link
Collaborator

Emiyaaaaa commented May 16, 2024

Thanks report, but I don't think this is a bug

Let's look some cases:

case 1

type Pet = {
 owner: { name: 'name1' }
      | { name: 'name2' ; foo: '' };
};
type Result = PickDeep<Pet, 'owner.name'>;

PickDeep should pick every union type, so result should indeed be

type Result = {
 owner: { name: 'name1' } | { name: 'name2' }
}

case 2

type Pet = {
 owner: { name: 'name1' }
      | { foo: '' };
};
type Result = PickDeep<Pet, 'owner.name'>;

PickDeep can not find key "name" in { foo: '' }, so result is

type Result = {
 owner: { name: 'name1' }
}

case 3

type Pet = {
 owner: { name: 'name1' }
      | null;
};
type Result = PickDeep<Pet, 'owner.name'>;

so according to case 2, PickDeep can not find key "name" in null, so result should be

type Result = {
 owner: { name: 'name1' }
}

This is the same as the current behavior

@Emiyaaaaa Emiyaaaaa removed the bug Something isn't working label May 16, 2024
@relsunkaev
Copy link
Author

relsunkaev commented May 16, 2024

That makes sense as to why it behaves like this but to me it seems like unexpected or maybe less ergonomic behavior.

My current use case is narrowing types on function inputs like so

type Owner = {
  name: string;
  age: number;
}

type Pet = {
  name: string;
  age: number;
  owner: Owner | null;
}

// I only use `name` and `owner.name`
function printGreeting(pet: PickDeep<Pet, "name" | "owner.name"> {
  if (pet.owner) {
    console.log(`Hi, my name is ${pet.name} and my owner is ${pet.owner.name}!`);
  } else {
    console.log(`Hi, my name is ${pet.name} and I'm looking for an owner!`);
  }
}

Right now the function would require pet to have an owner even though in the definition of Pet the owner can be null. This was unexpected because in my understanding the purpose of Pick is to narrow a type while PickDeep "changes" it.

@Emiyaaaaa
Copy link
Collaborator

Okay, you convinced me. maybe we should keep union in PickDeep to ensure type safety.

What do you think? @sindresorhus

@sindresorhus
Copy link
Owner

What would "case 2" become if so?

@relsunkaev
Copy link
Author

relsunkaev commented May 20, 2024

@sindresorhus I actually thought about this a bit when writing my response. This behavior is not consistent with how Pick or property access on union types works in TypeScript and can also be considered unintuitive:

type Pet = {
  name: string;
  owner: { name: 'name1' } | { foo: '' };
};

declare const pet: Pet;

function onlyNeedsOwnerName(pet: PickDeep<Pet, "owner.name">) {
  ...
}

onlyNeedsOwnerName(pet)
// ^ will result in an error

Again, in this case PickDeep changes the type. Pet is not compatible with PickDeep<Pet, "owner.name"> which some people may find surprising.

@Emiyaaaaa
Copy link
Collaborator

What would "case 2" become if so?

that will be

type Result = {
 owner: { name: 'name1' } | { foo: "" }
}

It's look like confused, I am beginning to waver on whether need do this for "type safety"

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

3 participants