Playing around with generics and options #12
Conversation
|
Sounds cool! Leaving the API open for more options (maybe an optional custom |
| ): Observable<DocumentChange<T>[]> { | ||
| return fromCollectionRef(query, { includeMetadataChanges: true }).pipe( | ||
| const events = options.events || ALL_EVENTS; | ||
| const snapshotListenOptions = options.snapshotListenOptions ?? DEFAULT_SNAPSHOT_LISTEN_OPTIONS; |
There was a problem hiding this comment.
may want to spread these instead, so that if someone specifies some options but not others, it doesn't get confused:
const snapshotListenOptions = {
...DEFAULT_SNAPSHOT_LISTEN_OPTIONS,
...options.snapshotListenOptions,
};
|
@davideast – any chance of getting something like this in the next (breaking) version, and available in angular/angularfire#3402? This would resolve the "double updates" issue. I'm happy to contribute a PR, with some guidance. |
|
@jits If you'd like to create an proposal first and open it as an issue for discussion I would love to review it with you. I'm very much in favor! |
|
@davideast – sounds good! Will do, in the next couple of days. |
|
@davideast – #84 🎉 |
In playing around with the ergonomics of the current API and looking at our feature requests, I thought it would be a good pattern to break our second argument into an options object. We have a chance to break since we're going 5.0. Here's a first pass on what it might look like.
collectionChanges(ref)(default all events,includeMetadatatrue)collectionChanges(ref, { events: ['added'] })collectionChanges(ref, { events: ['added'], snapshotListenOptions: { includeMetadata: false } })Also I've typed the
idFieldoption a little better, given our recent generics work.collectionData(ref) => Observable<DocumentData[]>collectionData(ref, { idField: 'id' }) => Observable<DocumentData[]>collectionData<Foo>(ref, { idField: 'id' }) => Observable<Foo[]>with typescript erroring ifFoodoesn't contain anid: stringfield.I don't think I could force it to be
DocumentData & { id: string }(like we do in AngularFire [but some folk don't like]) in the case that they don't explicitly pass a generic, unless we hurt the DevExp with something likecollectionData<Foo, 'id'>(ref, { idField: 'id' }), which I'd rather not do. This would also forbid us from being flexible with options and adding something likecollectionData<Foo>(ref, { idField: 'id', referenceField: 'ref', metadataField: 'meta' }), etc. which have been requested.WDYTT? If you're both amenable, I'll draft it up for the other functions too.