chore(typings): add typings for buffer and window based methods #1311
Conversation
| @@ -23,6 +23,10 @@ export function buffer<T>(closingNotifier: Observable<any>): Observable<T[]> { | |||
| return this.lift(new BufferOperator<T>(closingNotifier)); | |||
| } | |||
|
|
|||
| export interface BufferSignature<T> { | |||
| (closingNotifier: Observable<any>): Observable<T[]>; | |||
There was a problem hiding this comment.
closingNotifier calls through to subscribeToResult, so technically this could be ObservableInput<T>. That doesn't really make sense for the more finite values from Promise<T>, Iterable<T> or Array<T>. So I think the best option is to explicitly leave these as just Observable<T>.
There was a problem hiding this comment.
I think this is related to #1246. Probably we want to support return values other than observable - but I'd like to take care it as separate changes set include type update / test cases to ensure its behavior if necessary.
david-driscoll
force-pushed
the
typings/buffer-window
branch
from
f55945c
to
f53c169
Compare
| return this.lift(new BufferCountOperator<T>(bufferSize, startBufferEvery)); | ||
| } | ||
|
|
||
| export interface BufferCountSignature<T> { |
There was a problem hiding this comment.
Compare to previous type definitions having n-number of param (combinelatest, etcs) these kind of operators have single signature only. Is it necessarily required to export signature then import in coreoperators?
There was a problem hiding this comment.
Sadly yes because we can't use typeof bufferCount, because bufferCount is a function that form of casting isn't supported.
There was a problem hiding this comment.
OK, got point. Thanks for clarification.
There was a problem hiding this comment.
One way to work around extra import is to use interface merging: export interface bufferCount. Now no extra import is required for BufferSignature, bufferCount acts as both function and signature. The benefit is marginal however.
There was a problem hiding this comment.
Yes, I don't think this is serious issue need to be addressed. Mostly curious question.
|
Change looks good to me, (need to be rebased though) |
david-driscoll
force-pushed
the
typings/buffer-window
branch
from
f53c169
to
30217c4
Compare
|
done. |
|
Merged with 51add30, thanks @david-driscoll . |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
will probably need a rebase after #1292