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

Type that adds a property #177

Closed
mightyiam opened this issue Sep 3, 2020 · 9 comments
Closed

Type that adds a property #177

mightyiam opened this issue Sep 3, 2020 · 9 comments

Comments

@mightyiam
Copy link

Would this type be welcome?

Similar to Merge, but for a single property.

Not certain regarding the name.

export type Add<T, V, N extends string | symbol> = T & { [P in N]: V }

@gomain

@quezak
Copy link
Collaborator

quezak commented Sep 3, 2020

@mightyiam is there a difference why you can't use Merge directly?

// also, you say "for a single property" but your example can add multiple properties, was this intended? For example Add<{}, number, 'a' | 'b' | 'c'> or even Add<{}, number, string>

@gomain
Copy link

gomain commented Sep 3, 2020

It's for a usecase where N is inferred from a value

const AddProperty = <T, V, N extends string>(object: T, value: V, name: N): Add<T,V,N> = {
  return {
    ...object,
    [name]: value
  }
}

@quezak
Copy link
Collaborator

quezak commented Sep 3, 2020

You could still write the above example's return type as Merge<T, { [k in N]: V }>.

@krzkaczor what do you think, is it worth sucha a "convenience alias"? I'm not 100% sure, because this really is just a different way to call Merge in a specific case where all the new properties have the same type. But also I'm not against it 😃 though I'd reorder the args to <T, KeyT, ValueT> since it's more intuitive

@mightyiam
Copy link
Author

It's meant for a single property. Does anyone happen to know how that could be implemented?

@quezak
Copy link
Collaborator

quezak commented Sep 3, 2020

It's meant for a single property. Does anyone happen to know how that could be implemented?

Exactly the same, only with some more complex requirements on what N can be (in particular that it can't have multiple values, for example using #100). But I don't think that's a problem, since your proposal actually works correctly when N / KeyT includes multiple properties.

@papb
Copy link

papb commented Sep 3, 2020

Hello, I think the following would give a better intellisense when hovering on VSCode:

export type Add<T, V, N extends string | symbol> = { [P in keyof T | N]: P extends N ? T[P] & V : T[P] }

@gomain
Copy link

gomain commented Sep 4, 2020

Hello, I think the following would give a better intellisense when hovering on VSCode:

export type Add<T, V, N extends string | symbol> = { [P in keyof T | N]: P extends N ? T[P] & V : T[P] }

Shouldn't it be

type Add<T, V, N extends string | symbol> = { [P in keyof T | N]: P extends N ? V : T[P] }

@papb
Copy link

papb commented Sep 4, 2020

@gomain The type proposed by OP was

type Add<T, V, N extends string | symbol> = T & { [P in N]: V }

Therefore to be equivalent, it should be the way I proposed it.

@krzkaczor
Copy link
Collaborator

Thanks for the proposal but I think it's better to keep API smaller with only Merge type. We are already having problems with the number of types in this lib.

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

5 participants