Conversation
|
I'm fine with this PR, but is there any way to include a ts build/check step to |
There was a problem hiding this comment.
This looks better than what's in DefinitelyTyped.
Have you considered using overloading instead of a union of queries? This would prevent you from e.g. using an ArrayOperator on something that's not an array.
declare function update<T>(data: ReadonlyArray<T>, query: update.ArrayOperator<T>): ReadonlyArray<T>
declare function update<T>(data: ReadonlySet<T>, query: { $add: ReadonlyArray<T> } | { $remove: ReadonlyArray<T> }): ReadonlySet<T>
declare function update<K, V>(data: ReadonlyMap<K, V>, query: { $add: ReadonlyArray<[K, V]> } | { $remove: ReadonlyArray<K> }): ReadonlyMap<K, V>
declare function update<K extends string, T extends { readonly [key in K]: boolean }>(data: T, query: { $toggle: ReadonlyArray<K> }): T;
...etc......although I can't figure out how you would strongly-type nested queries without conditional types (microsoft/TypeScript#12424).
If you're interested in testing, dtslint can be run outside of DefinitelyTyped. (But make sure to remove the // Type definitions for immutability-helper header or it will think you're on DefinitelyTyped.)
index.d.ts
Outdated
| // Type definitions for immutability-helper | ||
| // Project: Immutability helper | ||
|
|
||
| export default update |
There was a problem hiding this comment.
In the source code this is module.exports =, so it should be declared as export =.
See https://github.com/DefinitelyTyped/DefinitelyTyped#a-package-uses-export--but-i-prefer-to-use-default-imports-can-i-change-export--to-export-default
There was a problem hiding this comment.
Cool, thanks
|
Oh, thanks. I'll get over everything a covert it into a commit soon. And yes, I have seen microsoft/TypeScript#12424 while researching a way to provide stronger types for the lib. I believe overloading is way better than having unions, thanks for that. But yeah, without conditional types we are indeed loosing some information about the generic type |
2 similar comments
|
@andreiglingeanu It looks good, but it doesn't seem like it will allow you to push to an array that's nested as a property? You would have to include |
|
Oh, my bad! For some kind of a reason I thought the array would be only on the first level. I'll fix the typings, thanks. |
|
@Andy-MS pushed the change. Now arrays, Map & Sets can be nested into objects. Does that look okay? |
3 similar comments
index.d.ts
Outdated
| | {$splice: Array<[number, number]>} | ||
| | {$splice: Array<[number, number]>} | ||
|
|
||
| type MapAndSetOperators<T> = {$add: any[]} | {$remove: string[]} |
There was a problem hiding this comment.
For a Map, shouldn't this be $remove: K[] and $add: [K, V][]?
Similarly, for a Set you would remove/ add T, not string.
|
Yeah, missed that. A strong limitation is that I won't be able to type properly the nested |
1 similar comment
index.d.ts
Outdated
| | Tree<T> | ||
| | ObjectOperators<T> | ||
| | ArrayOperators<T> | ||
| | SetOperators<T> |
There was a problem hiding this comment.
This would just be ArrayOperators<any> and SetOperators<any> since T here is the type of the object itself, not the type of its elements.
index.d.ts
Outdated
| query: ArrayOperators<T>, | ||
| ): ReadonlyArray<T> | ||
|
|
||
| declare function update<T>(data: T, query: Query<T>): object |
There was a problem hiding this comment.
This should be the last overload so that the ReadonlySet and ReadonlyMap ones are tried first.
index.d.ts
Outdated
| | {$toggle: Array<keyof T>} | ||
| | {$unset: Array<keyof T>} | ||
| | {$merge: Partial<T>} | ||
| | {$apply: (old: T) => any} |
|
@Andy-MS Thanks, learned a lot :) |
|
Awesome! Thanks for this. I'm going to add |
|
Also I imagine that #79 can be closed, is that correct? |
|
Great, published to |
3 participants
Fixes #76
This will:
updatefunctionMapAndSetOperatorsgenericcc. @kolodny