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

Add Diff and Spread types #7

Closed
wants to merge 2 commits into from

Conversation

EdwardDrapkin
Copy link

No description provided.

@EdwardDrapkin EdwardDrapkin changed the title Add Diff<T,V> Add Diff<T,V> and Spread<T,V> Mar 14, 2019
@sindresorhus
Copy link
Owner

I just added some contribution guidelines. Would you be able to review them? I'm also interested in feedback if anything there is unclear or could be improved.

https://github.com/sindresorhus/type-fest/blob/master/.github/contributing.md

@@ -84,3 +84,23 @@ const ab: Merge<Foo, Bar> = {a: 1, b: 2};
```
*/
export type Merge<FirstType, SecondType> = Omit<FirstType, Extract<keyof FirstType, keyof SecondType>> & SecondType;

/**
* Diffs two objects.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this to the readme too?

*
* @example `type Safe = Diff<AllProperties, UnsafeProperties>`
*/
export type Diff<T extends {}, V extends {}> = {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use more descriptive type names than T and V? See the other types for inspiration.

*
* Given objects with types T and V, returns an object that has all the keys in T that do not also exist in V.
*
* @example `type Safe = Diff<AllProperties, UnsafeProperties>`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you include a more comprehensive example? Should also include the import statement.

/**
* Returns a type modeling the result of spreading two objects together.
*
* Given objects with types T and V, returns an object that has a type that represents {...T, ...V}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from Merge?

* Returns a type modeling the result of spreading two objects together.
*
* Given objects with types T and V, returns an object that has a type that represents {...T, ...V}
* @example `const a: Spread<X, Y> = { ...x, ...y }`
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't TS already infer the new type correctly from object-spread? Maybe I'm missing the use-case for this type.

@sindresorhus sindresorhus changed the title Add Diff<T,V> and Spread<T,V> Add Diff<T,V> and Spread<T,V> types Mar 14, 2019
@sindresorhus sindresorhus changed the title Add Diff<T,V> and Spread<T,V> types Add Diff<T, V> and Spread<T, V> types Mar 14, 2019
@sindresorhus sindresorhus changed the title Add Diff<T, V> and Spread<T, V> types Add Diff and Spread types Mar 14, 2019
@sindresorhus
Copy link
Owner

Ping :)

@sindresorhus
Copy link
Owner

@CvX @BendingBender Do you think these types are something we should add?

@CvX
Copy link
Contributor

CvX commented Apr 19, 2019

I prefer to extract types from existing code. I find it difficult to judge if given type will find a use in the future. 😅 Some real-world examples would go a long way.

@BendingBender
Copy link
Contributor

@sindresorhus I'm not sure about this PR. Like @CvX I have difficulties imagining using types like this. They are clever solutons but usually a simpler "dumb" solution is easier to understand and is at the same time obvious for every dev who reads it. I'm sure that these types can be useful, especially in the context of ambient declarations but I have yet to encounter a use case for them.

As you've already pointed out, this library is mostly for types that are useful for your projects (#3 (comment)). So what's the point adding types that you can't even imagine to use?

@alixaxel
Copy link

alixaxel commented Nov 4, 2019

I think I have a use case for where this would be helpful. In a ORM I built, I have a few decorators that can potentially be attached for a model, and will automatically populate id + createdAt + updatedAt.

However, in some models, it doesn't make sense to have either one of those:

Omit<T, keyof (Identified & Timestamped)>
&
PartialMaybe<T, Identified>
&
PartialMaybe<T, Timestamped>

For context:

type PartialMaybe<T, O> = T extends O ? Partial<O> : {};

interface Identified {
  id: string;
}

interface Timestamped {
  createdAt: firestore.Timestamp;
  updatedAt?: firestore.Timestamp;
}

So in practice this will conditionally (if they exist) make Identified and Timestamped types optional.

@Richienb
Copy link
Contributor

@sindresorhus My use case for Spread is when I'm trying to document return types without creating new interfaces.

interface returnTypes {
	somePropertyToExtend: string
}

function someFunction(): Spread<returnTypes, {
	somethingElseThatIsNeeded: string
}>

function anotherFunction(): Spread<returnTypes, {
	anotherThingThatIsNeeded: string
}>

@sindresorhus
Copy link
Owner

It would be better to open a new issue to discuss this.

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

Successfully merging this pull request may close these issues.

6 participants