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

Using spy on a TS > 4.3.x project causes CombineLatest ref to be function instead of Observable #15

Closed
LironHazan opened this issue Jul 21, 2022 · 11 comments · Fixed by #18

Comments

@LironHazan
Copy link
Collaborator

@pauleustice

Thinking out load:
With TS 4.4.x following line fails:
service.combinedProviderObservable$.subscribe((value)

TypeError: service.combinedProviderObservable$.subscribe is not a function

I've printed the service for 4.4.x and got:
'**combinedProviderObservable$**': [Function (anonymous)],

And for 4.3.x we get:

     'combinedProviderObservable$': Observable {
        _isScalar: false,
        source: Observable {
          _isScalar: false,
          source: [Observable],
          operator: [CombineLatestOperator]
        },
        operator: MapOperator { project: [Function (anonymous)], thisArg: undefined }
      }

When removing the use of spy it works in 4.4.x,
In TS 4.4.x: When using spy CombineLatest returns a function ref and without the spy the CombineLatest returns an observable.
Won't happen in TS 4.3.x
Wow I don't have a clue,
I'm unfamiliar as well with the spy related code, but what I can do in this repo is adding the test with rxjs that would fail and then we can debug on ts-mockito source

@LironHazan LironHazan changed the title Using spy on a TS 4.4.x project causes CombineLatest ref to be treaded as function instead of Observable Using spy on a TS 4.4.x project causes CombineLatest ref to be function instead of Observable Jul 21, 2022
@pauleustice
Copy link

For reference, previous discussions relating to this issue can be found starting with this comment.

@LironHazan
Copy link
Collaborator Author

@pauleustice -->
https:/TypeStrong/ts-mockito/tree/15-combinelatest-as-function-issue/test/issue15
Added the tests, fails for same reason and now we can debug,
I'll try to look into it later on today

@LironHazan LironHazan changed the title Using spy on a TS 4.4.x project causes CombineLatest ref to be function instead of Observable Using spy on a TS > 4.3.x project causes CombineLatest ref to be function instead of Observable Jul 21, 2022
@pauleustice
Copy link

pauleustice commented Jul 21, 2022

@LironHazan Cool! I've added some logging into Mock.js (which Spy.js extends).

Mocker.prototype.processProperties:

if (descriptor.get) {
    console.log('get');
    _this.createPropertyStub(name);
    _this.createInstancePropertyDescriptorListener(name, descriptor, obj);
    _this.createInstanceActionListener(name, obj);
}
else if (typeof descriptor.value === "function") {
    console.log('function');
    _this.createMethodStub(name);
    _this.createInstanceActionListener(name, obj);
}
else {
  console.log('neither');
}

When it iterates over the class fields on ExampleService, the following happens:

  console.log
    observable$

    neither
    
  console.log
    providerOneObservable$

    neither
    
  console.log
    combinedProviderObservable$

    function

That's why the stub is being added. Now we need to work out what the difference in TS version is, that means the combineLatest Observable is a function.

EDIT: To clarify, with TS 4.3.5 the log is:

  console.log
    combinedProviderObservable$

    neither

And as such, it does not get stubbed.

@pauleustice
Copy link

@LironHazan I haven't had a chance to do too much more on this, but I've just attempted to isolate the issue further by using ts-mockito outside of Jest. I created a basic TS file, which behaves as expected:

import { Observable } from 'rxjs';
import { BehaviorSubject, combineLatest } from 'rxjs';

import { spy } from 'ts-mockito';

class SimpleClass {
  observable$ = new BehaviorSubject('one');
  observableTwo$ = new BehaviorSubject('two');

  localCombinedObservable$: Observable<string[]> = combineLatest([
    this.observable$,
    this.observableTwo$,
  ]);
}

const simple = new SimpleClass();

spy(simple);

simple.localCombinedObservable$.subscribe(([ one, two ]) => {
  console.log(one, two);
});

This suggests to me that it isn't Typescript / ts-mockito exclusively, more like Jest, ts-jest or something else... 🤔

@LironHazan
Copy link
Collaborator Author

LironHazan commented Jul 25, 2022

@pauleustice Interesting and a different angle than what I thought,
I found that MockableFunctionsFinder recognised the following code as a function

        this.localCombinedObservable$ = (0, rxjs_1.combineLatest)([
            this.observable$,
            this.observableTwo$,
        ]).pipe((0, operators_1.map)(function (_a) {
            var first = _a[0], another = _a[1];
            return "".concat(first, "/").concat(another);
        }));

This is the transpiled code as string when running version > 4.3.x

And for 4.3.5 its:

this.localCombinedObservable$ = rxjs_1.combineLatest([
            this.observable$,
            this.observableTwo$,
]).pipe(operators_1.map(function (_a) {
   var second = _a[0], third = _a[1];
   return second + "/" + third;
}));

I tried to fix the "functionNameRegex" but that didn't work,
But now I don't think that's the issue after seeing your isolated example

@LironHazan
Copy link
Collaborator Author

@pauleustice btw I tried running with karma instead and still fails for same reason,
TypeError: service.combinedProviderObservable$.subscribe is not a function

@pauleustice
Copy link

pauleustice commented Jul 25, 2022

Interesting... so it's probably not Jest then! So in your examples above, did MockableFunctionsFinder not show up the this.localCombinedObservable in 4.3.5? Both of them yield the same result so it's weird that it has issues with the first. I probably won't have time to look at this again today but when I next do, I'll look at that regex too.

@LironHazan
Copy link
Collaborator Author

@pauleusticeI it seems like my debugger was out of sync!
fixing the regex fixed the issue!
but there are a lot of other tests which fails now so I need to dig into it

@pauleustice
Copy link

Oh wow, amazing work! Let me know if I can help, I can make some time tomorrow!

@johanblumenberg
Copy link

I fixed it in my fork: johanblumenberg@08979c2

I opened a PR to try to fix it in this repository as well, but it wasn't straightforward. See details in #17.

@LironHazan
Copy link
Collaborator Author

@pauleustice I started to resolve it using a parser #17 (comment) but I want to try to merge @johanblumenberg PR instead

@LironHazan LironHazan linked a pull request Jul 26, 2022 that will close this issue
johanblumenberg added a commit to johanblumenberg/ts-mockito that referenced this issue Jul 31, 2022
johanblumenberg added a commit to johanblumenberg/ts-mockito that referenced this issue Aug 1, 2022
johanblumenberg added a commit to johanblumenberg/ts-mockito that referenced this issue Aug 12, 2022
johanblumenberg added a commit to johanblumenberg/ts-mockito that referenced this issue Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants