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

Expand operator behavior #766

Closed
kwonoj opened this issue Nov 23, 2015 · 6 comments
Closed

Expand operator behavior #766

kwonoj opened this issue Nov 23, 2015 · 6 comments

Comments

@kwonoj
Copy link
Member

kwonoj commented Nov 23, 2015

Took example snippet from RxJS 4,

var source = Rx.Observable.return(42)
    .expand(function (x) { return Rx.Observable.return(42 + x); })
    .take(5);

var subscription = source.subscribe(console.log, console.log, ...);

RxJS4

Next: 42
Next: 84
Next: 126
Next: 168
Next: 210
Completed

RxJS Next

RangeError: Maximum call stack size exceeded
    at ExpandSubscriber._next 
@benlesh
Copy link
Member

benlesh commented Nov 23, 2015

This seems to be more related to take, as when I remove take it works fine.

cc/ @trxcllnt

@kwonoj
Copy link
Member Author

kwonoj commented Nov 25, 2015

Took a look this bit more, this is behavior of expand. Internally it recursively calling _next() to expand, having 2 problems

  • for infinite expand with subscription, recursive call causes stack overflow
  • in case of scalar value result via projection at
 if (result._isScalar) {
          this._next(result.value);
 }

it infinitely continues while there isn't terminating condition.

@benlesh
Copy link
Member

benlesh commented Nov 30, 2015

for infinite expand with subscription, recursive call causes stack overflow

expand is apparently missing the scheduler argument. We should still default to "no scheduler", which means immediate execution (and potential stack overflow).

@kwonoj
Copy link
Member Author

kwonoj commented Nov 30, 2015

expand is apparently missing the scheduler argument

: compare to RxJS4, yes it does. I'll try changes for this.

(and potential stack overflow)

: I think this is similar case to #651, eventually might need some mechanism to avoid stack overflows.

@benlesh
Copy link
Member

benlesh commented Nov 30, 2015

eventually might need some mechanism to avoid stack overflows.

That mechanism is allowing users to provide a scheduler.

The problem is that any mechanism (scheduling) to avoid stack overflows also: 1. is less performant, and 2. makes debugging with call stacks gnarly. ... those are two of our top goals.

kwonoj added a commit to kwonoj/rxjs that referenced this issue Nov 30, 2015
@kwonoj
Copy link
Member Author

kwonoj commented Dec 1, 2015

I'm closing this issue since scheduler parameter is being tracked in #841 .

@kwonoj kwonoj closed this as completed Dec 1, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 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

No branches or pull requests

2 participants