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

lift signature issue returns on new typescript version #2726

Closed
ritox842 opened this issue Jul 6, 2017 · 8 comments
Closed

lift signature issue returns on new typescript version #2726

ritox842 opened this issue Jul 6, 2017 · 8 comments

Comments

@ritox842
Copy link

ritox842 commented Jul 6, 2017

On version 5.4.2 there's a bug fix:

Subject: lift signature is now appropriate for stricter TypeScript 2.4 checks (#2722) (9804de7)

I'm using typescript version 2.4.1 and I'm getting this bug again...

What can I do?

@ericeslinger
Copy link

yeah, I'm having the same issue - rolling back to [email protected] for now.

@kwonoj
Copy link
Member

kwonoj commented Jul 7, 2017

If you're experiencing this still, please provide minial repo have single rxjs dependency can repro this issues.

@ericeslinger
Copy link

super-minimal test case here: https://github.com/ericeslinger/lift-example

git clone https://github.com/ericeslinger/lift-example.git
cd lift-example && npm i && npm test

All I really do is implement lift thusly:

export class ExtendedObservable<T> extends Observable<T> {
  lift(operator) {
    const observable = new ExtendedObservable();
    observable.operator = operator;
    return observable;
  }
}

which isn't precisely what I'm doing in my actual work, but seems like a pretty minimal reproduction.

this is confirmed to work when I install [email protected] and fail under 2.4.1 (to test in the repo, npm i [email protected] && npm test )

@kwonoj
Copy link
Member

kwonoj commented Jul 7, 2017

@ericeslinger your example isn't cases of lib's incorrect type signature, it's expected TS behavior.

Note TS 2.4.1 enables strict generic check, reason why your code works on prior version of compiler. Signature of base lift is lift<R>(operator: Operator<T, R>): Observable<R>, and inherited lift is inferred by compiler as life(operator:any): ExtendedObservable<{}>, so there is type incompatibility occurs.

Correcting it by explicit casting like

lift<T>(operator): Observable<T> {
    const observable = new ExtendedObservable() as Observable<T>;
    (observable as any).operator = operator;
    return observable;
  }

resolves issue. (Note: above snippet is only for illustration)

I'll leave this issue opened bit more to see if there's any remaining issue around library itself - meanwhile, anyone experience similar issues please check 2 things

  • Is there any dependency still have old version of Rx in somewhere?
    : you have to physically check node_modules for this. To isolate this, simplest way is create minimal reproducible repo.

  • Is this caused by library code?
    : If so, it should happen by using variant of observable inherited object, like Subject. If not, check if user code have compatible types.

@GulinSS
Copy link

GulinSS commented Jul 12, 2017

See #2722 (comment)
Looks like need update .d.ts also

@kwonoj
Copy link
Member

kwonoj commented Jul 12, 2017

@GulinSS d.ts type definition is auto-generated and already updated.

image

Pleasd check above comment (#2726 (comment)) , and share reproducible repo for code ensure if there is no dupe pkg, or user code has incompatible types.

@GulinSS
Copy link

GulinSS commented Jul 13, 2017

@kwonoj, did deeper investigation, the issue was in ionic-native dep which referenced to older rxjs version

@kwonoj
Copy link
Member

kwonoj commented Jul 17, 2017

as I do not see further report related with library code itself, I'm closing this issue for now. Anyone experiences similar issue and confirmed if it's not coming from old pkg version or user code, feel freely create new issue with reproducible repo.

@kwonoj kwonoj closed this as completed Jul 17, 2017
@ReactiveX ReactiveX locked and limited conversation to collaborators Aug 14, 2017
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

4 participants