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

[TS Typing] allow mergeAll on Observable<T[]> like mergeMap(x => x) allows #2372

Closed
Dorus opened this issue Feb 14, 2017 · 7 comments · Fixed by #2374
Closed

[TS Typing] allow mergeAll on Observable<T[]> like mergeMap(x => x) allows #2372

Dorus opened this issue Feb 14, 2017 · 7 comments · Fixed by #2374

Comments

@Dorus
Copy link

Dorus commented Feb 14, 2017

On an observable of an array, it's possible to call .mergeMap(x => x) instead of .mergeMap(x => Rx.Observalble.from(x)).

While .mergeMap(x => x) should be identical to mergeAll(), in TS it's not allowed to call mergeAll() on Observable<T[]>, but .mergeMap(x => x) is allowed.

@Dorus Dorus changed the title [TS Typing] allow mergeAll on Observable<T[]> like `` [TS Typing] allow mergeAll on Observable<T[]> like mergeMap(x => x) allows Feb 14, 2017
@kwonoj
Copy link
Member

kwonoj commented Feb 14, 2017

I'm not sure if I understood issue correctly. Can you provide detail with snippet example for clarity?

@Dorus
Copy link
Author

Dorus commented Feb 14, 2017

Code sample:

Rx.Observable.of([1,2,3]).mergeMap(x => x)

gives:

1
2
3

and

Rx.Observable.of([1,2,3]).mergeAll()

gives

[ts] Property 'subscribe' does not exist on type 'number[]'.

This should produce the same result, an observable of 1,2,3, but in Ts, the second one wont compile.

@kwonoj
Copy link
Member

kwonoj commented Feb 14, 2017

Maybe I'm lost, but isn't this in above code snippet?

@kwonoj
Copy link
Member

kwonoj commented Feb 14, 2017

Oh, now I see. Sorry for confusions.

@Dorus
Copy link
Author

Dorus commented Feb 14, 2017

@kwonoj Sorry for the confusion, i should have added subscribe to the observable. My point is that the second line will return number[] instead of Observable<number>

@kwonoj
Copy link
Member

kwonoj commented Feb 14, 2017

Actually I was surprised we have updated concatAll for similar reason but forgot mergemap. #1436 :p think you can blame me for that.

kwonoj added a commit to kwonoj/rxjs that referenced this issue Feb 14, 2017
trxcllnt pushed a commit to trxcllnt/rxjs that referenced this issue Feb 15, 2017
benlesh pushed a commit that referenced this issue Mar 27, 2017
* fix(timeout): Cancels scheduled timeout, if no longer needed

fixes #2134

* fix(timeoutWith): Cancels scheduled timeout, if no longer needed

* build(npm-scripts): update debug_mocha npm script for node 6

* fix(VirtualAction): Block rescheduled VirtualActions from executing their scheduled work.

VirtualActions are immutable so they can be inspected by the TestScheduler. In order to mirror

rescheduled stateful Actions, rescheduled VirtualActions shouldn't execute if they've been

rescheduled before execution.

* fix(timeout): Update timeout and timeoutWith to recycle their scheduled timeout actions.

The timeout and timeoutWith operators should dispose their scheduled timeout actions on

unsubscription. Also, given the new scheduling architecture, they can recycle their scheduled

actions so just one action is allocated per subscription.

* test(timeout): Add types to timeout and timeoutWith specs

* Fix merge conflicts

* Fix timeoutWith to work with new Subscriber leak fix.

* fix(timeout-spec): fix merge conflicts

* fix(Subscription): fold ChildSubscription logic into Subscriber to prevent operators from leaking ChildSubscriptions. (#2360)

The addition of ChildSubscription to fix #2244 accidentally introduced a different memory leak. Most operators that add and remove inner Subscriptions store the inner Subscriber instance, not the value returned by Subscription#add. When they try to remove the inner Subscription manually, nothing is removed, because the ChildSubscription wrapper instance is the one added to the subscriptions list.

Fixes #2355

* chore(publish): 5.1.1

* Ignore coverage

It's 5.5mb that people installing this don't need :)

* feat(AjaxObservable) : support 'PATCH' request type

Add support of the 'PATCH' request type based on the already existing 'PUT' request.

* fix(subscribeToResult): accept array-like as result

Accept array-like as a result to subscribe, so that
Observable.from and operators using subscribeToResult
have identical behaviour.

* chore(ajax.patch): Adds test for ajax.patch

* fix(forkJoin): add type signature for single observable with selector

Add type signature for using forkJoin with single observable as first parameter
and selector function as second parameter, so that typings would not prevent
usage which is permitted and properly handled by operator.

Closes #2347

* feat(webSocket): Add binaryType to config object

Add binaryType to config object, so that it is possible
to set that parameter on underlying socket before any
data emits happen.

Closes #2353

* fix(merge): return Observable when called with single lowerCaseO

Return Observable when merge is called with single lower case observable,
so that merge would always return Observable instance.

* fix(bindNodeCallback): emit undefined when callback has no success arguments

Emit undefined insteady of empty array by resulting Observable,
when callback function is called without success parameters.

Closes #2254

* chore(danger): update dangerfile to validate commit message

* chore(*): correctly scope disabled `max-line-length` tslint rule

The max line length is set to 150 in 'tslint.json'. In specific regions, it is
desirable to allow longer lines, so these regions should be wrapped in comments
like the following:

```js
// Max line length enforced here.

/* tslint:disable:max-line-length */
// Max line length NOT enforced here.
/* tslint:enable:max-line-length */   <-- CORRECT

// Max line length enforced here.
```

In many cases, the re-enabling comment incorrectly included `disable` instead of
`enable` (as shown below), which essentially keeps the `max-line-length`
**disabled** for the rest of the file:

```js
// Max line length enforced here.

/* tslint:disable:max-line-length */
// Max line length NOT enforced here.
/* tslint:disable:max-line-length */   <-- INCORRECT

// Max line length NOT enforced here.
```

This commit fixes these comments, so the `max-line-length` rule is properly
enforced in regions that don't need longer lines.

* fix(bindCallback): emit undefined when callback is without arguments

In resulting Observable emit undefined when callback is called without
parameters, instead of emitting empty array.

* fix(mergeAll): introduce variant support <T, R> for mergeMap

- closes #2372

* feat(windowTime): maxWindowSize parameter in windowTime operator

Adds new parameter in windowTime operator to control how much values given
window can emit.

Closes #1301

* docs(ObservableInput): add ObservableInput and SubscribableOrPromise descriptions

Add ObservableInput and SubscribableOrPromise interface descriptions,
as well as link these interfaces in type descriptions of operators,
so that users always know what kind of parameters they can pass to
used methods.

* fix(timeoutWith): update timeoutWith to work with new Subscriber leak fix changes
@lock
Copy link

lock bot commented Jun 6, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
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 a pull request may close this issue.

2 participants