Skip to content
This repository was archived by the owner on Jan 8, 2022. It is now read-only.

Conversation

@suneettipirneni
Copy link
Member

@suneettipirneni suneettipirneni commented Jan 2, 2022

Please describe the changes this PR makes and why it should be merged:

Closes #55

intersect and difference will provide a collection with a value type that is a union of the collection value type and value type of the collection to run against. Just like before, you need to have a matching key type.

For example:

Performing an intersection of Collection<string, string> onto a Collection<string, number> will yield the type Collection<string, number>;

Performing an difference of Collection<string, string> onto a Collection<string, number> will yield the type Collection<string, string | number>;

Performing an intersection or difference on a Collection<string, string> and a Collection<number, string> is an error since key types don't match.

Status and versioning classification:

  • Code changes have been tested, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating

@suneettipirneni suneettipirneni changed the title types: union value types on intersect and difference when types are different types: union value types on intersect and difference Jan 2, 2022
@Lioness100
Copy link

I don't understand why it's K | T - shouldn't it just be T?

for (const [k, v] of other) {
	if (this.has(k)) coll.set(k, v);
}

The values in the new collection are coming straight from collection other (with value type T). The values of the original collection do not come into play at all.

@suneettipirneni
Copy link
Member Author

I don't understand why it's K | T - shouldn't it just be T?

for (const [k, v] of other) {
	if (this.has(k)) coll.set(k, v);
}

The values in the new collection are coming straight from collection other (with value type T). The values of the original collection do not come into play at all.

This is true for intersect, I've gone ahead and fixed that. For difference it still uses values from both collections as seen here:

const coll = new this.constructor[Symbol.species]<K, V | T>();
for (const [k, v] of other) {
	if (!this.has(k)) coll.set(k, v);
}
for (const [k, v] of this) {
	if (!other.has(k)) coll.set(k, v);
}

@Lioness100
Copy link

Oh, sorry, I missed that 😅

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unnecessarily strict typings

5 participants