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

Strange behaviour of Observable.of([x]).map #1140

Closed
chrisprice opened this issue Jan 6, 2016 · 7 comments · Fixed by #1171
Closed

Strange behaviour of Observable.of([x]).map #1140

chrisprice opened this issue Jan 6, 2016 · 7 comments · Fixed by #1171
Labels
bug Confirmed bug

Comments

@chrisprice
Copy link
Contributor

I have a use case for creating an observable of arrays and came across some odd behaviour. The map function is invoked twice when using Observable.of(x) but not when using Observable.from([x]). Is this expected?

  it('should emit one value', function (done) {
    var calls = 0;
    Observable.of([42]).map(function (x) {
      expect(++calls).toBe(1);
      expect(x).toEqual([42]);
      return x;
    }).subscribe(function (x) {
      expect(++calls).toBe(2);
      expect(x).toEqual([42]);
    }, function (x) {
      done.fail('should not be called');
    }, done);
  });
Failures:
1) Observable.of should emit one value
  Message:
    Expected 2 to be 1.
  Stack:
    Error: Expected 2 to be 1.
        at ScalarObservable.<anonymous> (/Users/chris/RxJS/spec/observables/of-spec.js:51:23)
        at ScalarObservable.proto.map (/Users/chris/RxJS/dist/cjs/observable/ScalarObservable.js:61:45)
        at Object.<anonymous> (/Users/chris/RxJS/spec/observables/of-spec.js:50:25)
  Message:
    Expected 3 to be 2.
  Stack:
    Error: Expected 3 to be 2.
        at SafeSubscriber.<anonymous> (/Users/chris/RxJS/spec/observables/of-spec.js:55:23)
        at SafeSubscriber.tryCatcher [as _next] (/Users/chris/RxJS/dist/cjs/util/tryOrOnError.js:4:31)
        at SafeSubscriber.next (/Users/chris/RxJS/dist/cjs/Subscriber.js:89:18)
        at ScalarObservable._subscribe (/Users/chris/RxJS/dist/cjs/observable/ScalarObservable.js:44:24)

1999 specs, 1 failure

However, swapping Observable.of([42]) for Observable.from([[42]]) as below, doesn't trigger the failure.

  it('should emit one value', function (done) {
    var calls = 0;
    Observable.from([[42]]).map(function (x) {
      expect(++calls).toBe(1);
      expect(x).toEqual([42]);
      return x;
    }).subscribe(function (x) {
      expect(++calls).toBe(2);
      expect(x).toEqual([42]);
    }, function (x) {
      done.fail('should not be called');
    }, done);
  });
@chrisprice
Copy link
Contributor Author

Should have mentioned this was tested with master (d2e852).

@benlesh
Copy link
Member

benlesh commented Jan 6, 2016

@chrisprice ... currently there are optimizations for ScalarObservables. A ScalarObservable is any Observable that gives you a single, synchronous value. (in other words Observable.of(x)). If you map on a ScalarObservable, it will immediately perform the map and just give you a new ScalarObservable. This saves unnecessary computation when subscribing multiple times to that observable.

It's a powerful optimization, but it's up for debate, for sure.

from and of are two different animals. from will take any Observable, promise or iterable (arrays for example) and create an Observable from it's values. of will take arguments and make an Observable from them. Observable.of(a, b, c) and Observable.fromArray([a, b, c]) are effectively the same thing.

Generally, you shouldn't be creating side-effects in your map functions. Instead use do or subscribe.

@benlesh
Copy link
Member

benlesh commented Jan 6, 2016

In short...

  • Observable.of creates an Observable.
  • Observable.from converts to an Observable.
  • Observable.of(a) creates a ScalarObservable that has optimizations for synchronous operators like map, filter, scan and reduce.
  • Observable.of(a, b, c) creates an ArrayObservable that has no such optimizations.

@chrisprice
Copy link
Contributor Author

Thanks for the explanation, that does explain the behaviour I saw. The snippet I discovered this in caused me to pull my hair out -

Rx.Observable.of([
    {z: 1}
  ])
  .map((x) => 
    x.map((y) => (y.z += 'A', y))
  )
  .subscribe((x) => console.log(x))

Which logged out -

[[object Object] {
  z: "1AA"
}]

Now I agree that's an odd thing to be doing (I was just hacking some test data) and I also agree that map functions shouldn't contain side-effects, but it was totally unexpected behaviour!

@chrisprice
Copy link
Contributor Author

Actually I've been thinking about this and I'm not sure whether your explanation covers why the map function is called twice? I do understand that the map function will be called on initialisation rather than subscription but that's not what's being tested (I don't think!).

@benlesh
Copy link
Member

benlesh commented Jan 7, 2016

Yeah, there is a bug apparently:

let source = Rx.Observable.of(1)
  .map((x) => {
    console.log('hit');
    return x;
  });

logs:

"hit"
"hit"

Flagging this as a bug.

However it should go away if these optimizations are removed with #1142

@benlesh benlesh added the bug Confirmed bug label Jan 7, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Jan 11, 2016
- remove usage of ScalarObservable for optimization
- implementation of ScalarObservable still remain for possible further usage

closes ReactiveX#1142, ReactiveX#1150, ReactiveX#1140
@lock
Copy link

lock bot commented Jun 7, 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 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants