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

New errors with RxJS on 2.4.1 #16593

Closed
OliverJAsh opened this Issue Jun 17, 2017 · 35 comments

Comments

Projects
None yet
@OliverJAsh

OliverJAsh commented Jun 17, 2017

When using the TS 2.4.1 and RxJS 5.4.1 (latest), I get a new error that I didn't have before upgrading to using TS 2.4.1.

You may already be aware of this—it is an insiders build after all—but thought I should point it out just in case.

❯ yarn list typescript
└─ typescript@2.4.1-insiders.20170615

❯ yarn list rxjs
└─ rxjs@5.4.1

❯ tsc
node_modules/rxjs/Subject.d.ts(16,22): error TS2415: Class 'Subject<T>' incorrectly extends base class 'Observable<T>'.
  Types of property 'lift' are incompatible.
    Type '<R>(operator: Operator<T, R>) => Observable<T>' is not assignable to type '<R>(operator: Operator<T, R>) => Observable<R>'.
      Type 'Observable<T>' is not assignable to type 'Observable<R>'.
        Type 'T' is not assignable to type 'R'.
@DanielRosenwasser

This comment has been minimized.

Show comment
Hide comment
@DanielRosenwasser

DanielRosenwasser Jun 17, 2017

Member

CC @benlesh. This is actually a bug that's been caught. Subject<T>#lift<R> should be producing an Observable<R> but has always been producing an Observable<T>.

Check out https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#stricter-checking-for-generic-functions for details.

Member

DanielRosenwasser commented Jun 17, 2017

CC @benlesh. This is actually a bug that's been caught. Subject<T>#lift<R> should be producing an Observable<R> but has always been producing an Observable<T>.

Check out https://github.com/Microsoft/TypeScript/wiki/What's-new-in-TypeScript#stricter-checking-for-generic-functions for details.

@masaeedu

This comment has been minimized.

Show comment
Hide comment
@masaeedu

masaeedu Jun 17, 2017

Contributor

@OliverJAsh There was a long debate about this in ReactiveX/rxjs#1234. It's always seemed to me like the bug is in rxjs, in that that override in Subject shouldn't be there in the first place.

Contributor

masaeedu commented Jun 17, 2017

@OliverJAsh There was a long debate about this in ReactiveX/rxjs#1234. It's always seemed to me like the bug is in rxjs, in that that override in Subject shouldn't be there in the first place.

@DanielRosenwasser

This comment has been minimized.

Show comment
Hide comment
@DanielRosenwasser

DanielRosenwasser Jun 17, 2017

Member

So since then the signature has been changed to this:

lift<R>(operator: Operator<T, R>): Observable<T> 

I don't understand why back then it wasn't just

lift<R>(operator: Operator<T, R>): Observable<R> 

and it looks like the compiler finds the current signature to be questionable as well.

Member

DanielRosenwasser commented Jun 17, 2017

So since then the signature has been changed to this:

lift<R>(operator: Operator<T, R>): Observable<T> 

I don't understand why back then it wasn't just

lift<R>(operator: Operator<T, R>): Observable<R> 

and it looks like the compiler finds the current signature to be questionable as well.

@masaeedu

This comment has been minimized.

Show comment
Hide comment
@masaeedu

masaeedu Jun 17, 2017

Contributor

@DanielRosenwasser The problem is that they've been trying to use Subjects in a way that is fundamentally type-unsafe. I don't know if anything's changed in the year since that discussion happened, but in terms of the actual JS they want a Subject<T>, when transformed by an operator that takes Ts and produces Rs, to still produce a Subject (one that forwards all its values to this), instead of an Observable<R>. The reason they want to do this has something to do with two way communication in the websocket subject implementation.

None of this makes sense from the type system perspective, because it only makes sense to define lift for Observables. It just so happens that a Subject<T> is also an Observable<T>, but its implementation of lift<R> should be no different. The only sensible fix (and one which mirrors how the types are defined in Java/.NET) is to just delete this method and revisit what is going on in the websocket code. There's more details in the issue I linked, but the way they are using subjects can mask runtime bugs.

Alternatively, that entire method should be typed any-ly, because there is no sensible way to express in types what they are doing.

Contributor

masaeedu commented Jun 17, 2017

@DanielRosenwasser The problem is that they've been trying to use Subjects in a way that is fundamentally type-unsafe. I don't know if anything's changed in the year since that discussion happened, but in terms of the actual JS they want a Subject<T>, when transformed by an operator that takes Ts and produces Rs, to still produce a Subject (one that forwards all its values to this), instead of an Observable<R>. The reason they want to do this has something to do with two way communication in the websocket subject implementation.

None of this makes sense from the type system perspective, because it only makes sense to define lift for Observables. It just so happens that a Subject<T> is also an Observable<T>, but its implementation of lift<R> should be no different. The only sensible fix (and one which mirrors how the types are defined in Java/.NET) is to just delete this method and revisit what is going on in the websocket code. There's more details in the issue I linked, but the way they are using subjects can mask runtime bugs.

Alternatively, that entire method should be typed any-ly, because there is no sensible way to express in types what they are doing.

@DanielRosenwasser

This comment has been minimized.

Show comment
Hide comment
@DanielRosenwasser

DanielRosenwasser Jun 17, 2017

Member

Thanks for the explanation @masaeedu!

Couldn't this be solved by creating a TypeUnsafeSubject or something that lifts to Observable<any>?

Member

DanielRosenwasser commented Jun 17, 2017

Thanks for the explanation @masaeedu!

Couldn't this be solved by creating a TypeUnsafeSubject or something that lifts to Observable<any>?

@DanielRosenwasser

This comment has been minimized.

Show comment
Hide comment
@DanielRosenwasser
Member

DanielRosenwasser commented Jun 19, 2017

@DanielRosenwasser

This comment has been minimized.

Show comment
Hide comment
@DanielRosenwasser

DanielRosenwasser Jun 20, 2017

Member

I just saw this will be fixed for RxJS 6.0 thanks to ReactiveX/rxjs#2540 (@hearnden).

Member

DanielRosenwasser commented Jun 20, 2017

I just saw this will be fixed for RxJS 6.0 thanks to ReactiveX/rxjs#2540 (@hearnden).

@AneelaBrister

This comment has been minimized.

Show comment
Hide comment
@AneelaBrister

AneelaBrister Jun 27, 2017

Hi there, is the solution in the meantime to not go to Typescript 2.4? Thanks

AneelaBrister commented Jun 27, 2017

Hi there, is the solution in the meantime to not go to Typescript 2.4? Thanks

@michaeldaw

This comment has been minimized.

Show comment
Hide comment
@michaeldaw

michaeldaw Jun 27, 2017

@AneelaBrister Typescript version 2.3.4 seems to continue to work.

michaeldaw commented Jun 27, 2017

@AneelaBrister Typescript version 2.3.4 seems to continue to work.

@sflemingDCP

This comment has been minimized.

Show comment
Hide comment
@sflemingDCP

sflemingDCP Jun 27, 2017

What's the workaround here?

sflemingDCP commented Jun 27, 2017

What's the workaround here?

@ljmerza

This comment has been minimized.

Show comment
Hide comment
@ljmerza

ljmerza Jun 27, 2017

using typescript 2.3.4 or rxjs 6 alpha
npm install rxjs@6.0.0-alpha.0

ljmerza commented Jun 27, 2017

using typescript 2.3.4 or rxjs 6 alpha
npm install rxjs@6.0.0-alpha.0

@parkganesh

This comment has been minimized.

Show comment
Hide comment
@parkganesh

parkganesh Jun 28, 2017

Using typescript 2.3.4 worked. rxjs 6 alpha gave an unmet peer dependency with angular/router

parkganesh commented Jun 28, 2017

Using typescript 2.3.4 worked. rxjs 6 alpha gave an unmet peer dependency with angular/router

@lumiit-adam

This comment has been minimized.

Show comment
Hide comment
@lumiit-adam

lumiit-adam Jun 28, 2017

I uninstalled typescript 2.4.1 and installed 2.3.4, it was resolved.

lumiit-adam commented Jun 28, 2017

I uninstalled typescript 2.4.1 and installed 2.3.4, it was resolved.

@claudiuconstantin

This comment has been minimized.

Show comment
Hide comment
@claudiuconstantin

claudiuconstantin Jun 28, 2017

rxjs 6.0.0-alpha.0 still won't fix it for me, I'm still getting:

ERROR in [at-loader] ./node_modules/rxjs/Subject.d.ts:16:22
    TS2415: Class 'Subject<T>' incorrectly extends base class 'Observable<T>'.
  Types of property 'lift' are incompatible.
    Type '<R>(operator: Operator<T, R>) => Observable<T>' is not assignable to type '<R>(operator: Operator<T, R>) => Observable<R>'.
      Type 'Observable<T>' is not assignable to type 'Observable<R>'.
        Type 'T' is not assignable to type 'R'.

claudiuconstantin commented Jun 28, 2017

rxjs 6.0.0-alpha.0 still won't fix it for me, I'm still getting:

ERROR in [at-loader] ./node_modules/rxjs/Subject.d.ts:16:22
    TS2415: Class 'Subject<T>' incorrectly extends base class 'Observable<T>'.
  Types of property 'lift' are incompatible.
    Type '<R>(operator: Operator<T, R>) => Observable<T>' is not assignable to type '<R>(operator: Operator<T, R>) => Observable<R>'.
      Type 'Observable<T>' is not assignable to type 'Observable<R>'.
        Type 'T' is not assignable to type 'R'.
@floodedcodeboy

This comment has been minimized.

Show comment
Hide comment
@floodedcodeboy

floodedcodeboy Jun 28, 2017

I've used typescript 2.4.0 with rxjs 5.4.1 and that works fine.

floodedcodeboy commented Jun 28, 2017

I've used typescript 2.4.0 with rxjs 5.4.1 and that works fine.

@OliverJAsh

This comment has been minimized.

Show comment
Hide comment
@OliverJAsh

OliverJAsh Jun 28, 2017

@floodedcodeboy I just tested typescript 2.4.1 and rxjs 5.4.1, to no avail:

❯ npm list typescript
├── typescript@2.4.1

❯ npm list rxjs
├── rxjs@5.4.1

❯ tsc
node_modules/rxjs/Subject.d.ts(16,22): error TS2415: Class 'Subject<T>' incorrectly extends base class 'Observable<T>'.
  Types of property 'lift' are incompatible.
    Type '<R>(operator: Operator<T, R>) => Observable<T>' is not assignable to type '<R>(operator: Operator<T, R>) => Observable<R>'.
      Type 'Observable<T>' is not assignable to type 'Observable<R>'.
        Type 'T' is not assignable to type 'R'.

Sounds like the only solution is to upgrade to rxjs 6 alpha.

OliverJAsh commented Jun 28, 2017

@floodedcodeboy I just tested typescript 2.4.1 and rxjs 5.4.1, to no avail:

❯ npm list typescript
├── typescript@2.4.1

❯ npm list rxjs
├── rxjs@5.4.1

❯ tsc
node_modules/rxjs/Subject.d.ts(16,22): error TS2415: Class 'Subject<T>' incorrectly extends base class 'Observable<T>'.
  Types of property 'lift' are incompatible.
    Type '<R>(operator: Operator<T, R>) => Observable<T>' is not assignable to type '<R>(operator: Operator<T, R>) => Observable<R>'.
      Type 'Observable<T>' is not assignable to type 'Observable<R>'.
        Type 'T' is not assignable to type 'R'.

Sounds like the only solution is to upgrade to rxjs 6 alpha.

@sflemingDCP

This comment has been minimized.

Show comment
Hide comment
@sflemingDCP

sflemingDCP Jun 28, 2017

rxjs@6.0.0-alpha.0 and typescript 2.3.4 worked for me.

sflemingDCP commented Jun 28, 2017

rxjs@6.0.0-alpha.0 and typescript 2.3.4 worked for me.

@OliverJAsh OliverJAsh changed the title from New errors with RxJS on insiders version to New errors with RxJS on 2.4.x Jun 28, 2017

@OliverJAsh

This comment has been minimized.

Show comment
Hide comment
@OliverJAsh

OliverJAsh Jun 28, 2017

@sflemingDCP This issue is specifically about using rxjs with TypeScript 2.4.x.

OliverJAsh commented Jun 28, 2017

@sflemingDCP This issue is specifically about using rxjs with TypeScript 2.4.x.

@floodedcodeboy

This comment has been minimized.

Show comment
Hide comment
@floodedcodeboy

floodedcodeboy Jun 28, 2017

@OliverJAsh incorrect ... if you had read my post correctly ... I referenced 2.4.0 ... not 2.4.1 - make sure your package.json file references 2.4.0 exactly - not ^2.4.0 or ~2.4.0 which will upgrade you to 2.4.1

... I don't know why i bother really.

it also makes the title of this ticket incorrect as TypeScript 2.4.0 works with rxjs 5.4.1 - however TypeScript 2.4.1 does not work with rxjs 5.4.1.

If you want to use TypeScript 2.4.1 - then you will need to upgrade to rxjs 6 alpha ... I really hope you're not using that in production . :/

floodedcodeboy commented Jun 28, 2017

@OliverJAsh incorrect ... if you had read my post correctly ... I referenced 2.4.0 ... not 2.4.1 - make sure your package.json file references 2.4.0 exactly - not ^2.4.0 or ~2.4.0 which will upgrade you to 2.4.1

... I don't know why i bother really.

it also makes the title of this ticket incorrect as TypeScript 2.4.0 works with rxjs 5.4.1 - however TypeScript 2.4.1 does not work with rxjs 5.4.1.

If you want to use TypeScript 2.4.1 - then you will need to upgrade to rxjs 6 alpha ... I really hope you're not using that in production . :/

@OliverJAsh

This comment has been minimized.

Show comment
Hide comment
@OliverJAsh

OliverJAsh Jun 28, 2017

@floodedcodeboy You're correct. 2.4.0 works but 2.4.1 does not. I will update the issue title.

Confusingly, the releases page lists 2.4.1 as "2.4":

image

OliverJAsh commented Jun 28, 2017

@floodedcodeboy You're correct. 2.4.0 works but 2.4.1 does not. I will update the issue title.

Confusingly, the releases page lists 2.4.1 as "2.4":

image

@OliverJAsh OliverJAsh changed the title from New errors with RxJS on 2.4.x to New errors with RxJS on 2.4.1 Jun 28, 2017

@kitsonk

This comment has been minimized.

Show comment
Hide comment
@kitsonk

kitsonk Jun 28, 2017

Contributor

Confusingly, the releases page lists 2.4.1 as "2.4"

It isn't confusingly in as much as every minor release of TypeScript has followed that pattern for an extended period of time. Because of some internal challenges, from what I understand, #.#.0 will never be the final release of a version of TypeScript. The first patch public release will always be #.#.1 and tagged appropriately in npm.

Contributor

kitsonk commented Jun 28, 2017

Confusingly, the releases page lists 2.4.1 as "2.4"

It isn't confusingly in as much as every minor release of TypeScript has followed that pattern for an extended period of time. Because of some internal challenges, from what I understand, #.#.0 will never be the final release of a version of TypeScript. The first patch public release will always be #.#.1 and tagged appropriately in npm.

@Koslun

This comment has been minimized.

Show comment
Hide comment
@Koslun

Koslun Jun 28, 2017

Is it not be possible to simply override the type definition to ignore this error?

Upgrading to an alpha release does not seem like an ideal fix, in particular because I am not sure that our current dependencies truly support that, Angular 4 in particular. Just hope that RxJS 6 will release soon-ish so libraries such as Angular can incorporate it as the supported version. In the case of Angular that would happen either in the next major release scheduled for September or at the worst case the major release after that, around March 2018.

So I think not fixing this in another way could have a pretty tangible effect.

Koslun commented Jun 28, 2017

Is it not be possible to simply override the type definition to ignore this error?

Upgrading to an alpha release does not seem like an ideal fix, in particular because I am not sure that our current dependencies truly support that, Angular 4 in particular. Just hope that RxJS 6 will release soon-ish so libraries such as Angular can incorporate it as the supported version. In the case of Angular that would happen either in the next major release scheduled for September or at the worst case the major release after that, around March 2018.

So I think not fixing this in another way could have a pretty tangible effect.

@Koslun

This comment has been minimized.

Show comment
Hide comment
@Koslun

Koslun Jun 28, 2017

The issue in the RxJS repo: ReactiveX/rxjs#2539

Koslun commented Jun 28, 2017

The issue in the RxJS repo: ReactiveX/rxjs#2539

@shaamuji

This comment has been minimized.

Show comment
Hide comment
@shaamuji

shaamuji Jun 28, 2017

Replaced typescript from ^2.4.1 to 2.4.0 worked for me

shaamuji commented Jun 28, 2017

Replaced typescript from ^2.4.1 to 2.4.0 worked for me

@IrickNcqa

This comment has been minimized.

Show comment
Hide comment

IrickNcqa commented Jun 28, 2017

@ufthelp

This comment has been minimized.

Show comment
Hide comment
@ufthelp

ufthelp Jun 28, 2017

Thanks @floodedcodeboy . Your solution worked for me.

ufthelp commented Jun 28, 2017

Thanks @floodedcodeboy . Your solution worked for me.

@DanielRosenwasser

This comment has been minimized.

Show comment
Hide comment
@DanielRosenwasser

DanielRosenwasser Jun 28, 2017

Member

Hey all, you can get around this using the --noStrictGenericChecks flag. Sorry for the inconvenience.

Member

DanielRosenwasser commented Jun 28, 2017

Hey all, you can get around this using the --noStrictGenericChecks flag. Sorry for the inconvenience.

@aluanhaddad

This comment has been minimized.

Show comment
Hide comment
@aluanhaddad

aluanhaddad Jun 28, 2017

Contributor

Why not use an augmentation to resolve this temporarily without downgrading TypeScript or upgrading RxJS to an unstable version? The following works with TypeScript@2.4.1

// augmentations.ts
// TODO: Remove this when RxJS releases a stable version with a correct declaration of `Subject`.
import {Operator} from 'rxjs/Operator';
import {Observable} from 'rxjs/Observable';

declare module 'rxjs/Subject' {
  interface Subject<T> {
    lift<R>(operator: Operator<T, R>): Observable<R>;
  }
}
Contributor

aluanhaddad commented Jun 28, 2017

Why not use an augmentation to resolve this temporarily without downgrading TypeScript or upgrading RxJS to an unstable version? The following works with TypeScript@2.4.1

// augmentations.ts
// TODO: Remove this when RxJS releases a stable version with a correct declaration of `Subject`.
import {Operator} from 'rxjs/Operator';
import {Observable} from 'rxjs/Observable';

declare module 'rxjs/Subject' {
  interface Subject<T> {
    lift<R>(operator: Operator<T, R>): Observable<R>;
  }
}
@dhniels

This comment has been minimized.

Show comment
Hide comment
@dhniels

dhniels Jun 29, 2017

@claudiuconstantin im having thes same issue. still getting error TS2415 for Subject and WebSocketSubject with typescript 2.4.1 even after updating rxjs to 6.0.0-alpha.0

edit: may be a result of unmet peer dependencies?
running npm ls --depth=0 gives me these errors:
npm ERR! peer dep missing: rxjs@^5.0.1, required by @angular/core@4.2.4
npm ERR! peer dep missing: rxjs@^5.0.1, required by @angular/http@4.2.4
npm ERR! peer dep missing: rxjs@^5.0.1, required by @angular/router@4.2.4
npm ERR! peer dep missing: rxjs@^5.0.1, required by angular-in-memory-web-api@0.3.2

dhniels commented Jun 29, 2017

@claudiuconstantin im having thes same issue. still getting error TS2415 for Subject and WebSocketSubject with typescript 2.4.1 even after updating rxjs to 6.0.0-alpha.0

edit: may be a result of unmet peer dependencies?
running npm ls --depth=0 gives me these errors:
npm ERR! peer dep missing: rxjs@^5.0.1, required by @angular/core@4.2.4
npm ERR! peer dep missing: rxjs@^5.0.1, required by @angular/http@4.2.4
npm ERR! peer dep missing: rxjs@^5.0.1, required by @angular/router@4.2.4
npm ERR! peer dep missing: rxjs@^5.0.1, required by angular-in-memory-web-api@0.3.2

jt-nti added a commit to jt-nti/hyperledger-composer that referenced this issue Jun 29, 2017

Nasty hacks to work round yet another case of dependencies burning us:
Microsoft/TypeScript#16593

Work in progress (webpack isn't happy) and @sstone has an alternative
fix anyway

Signed-off-by: James Taylor <jamest@uk.ibm.com>
@Koslun

This comment has been minimized.

Show comment
Hide comment
@Koslun

Koslun Jun 29, 2017

edit: may be a result of unmet peer dependencies?

Unmet peer dependency errors in this instance just means that those angular dependencies require RxJS 5.0.1 or higher, but below 6. Angular could still work with RxJS higher than they have specified, as they often work with higher versions of TypeScript. But I think you're always at risk of some part of Angular not working or an arbitrary patch or minor version update breaking your build.

So use the solution proposed by @aluanhaddad instead, unless you for some other reason want to use the next version of RxJS already.

Koslun commented Jun 29, 2017

edit: may be a result of unmet peer dependencies?

Unmet peer dependency errors in this instance just means that those angular dependencies require RxJS 5.0.1 or higher, but below 6. Angular could still work with RxJS higher than they have specified, as they often work with higher versions of TypeScript. But I think you're always at risk of some part of Angular not working or an arbitrary patch or minor version update breaking your build.

So use the solution proposed by @aluanhaddad instead, unless you for some other reason want to use the next version of RxJS already.

@claudiuconstantin

This comment has been minimized.

Show comment
Hide comment
@claudiuconstantin

claudiuconstantin Jun 30, 2017

@aluanhaddad, nice work, all good if you directly use rxjsin your project. What if a certain package in your project also uses the "bad" rxjs? You'll also get the error from there and until the maintainer fixes the problem you're stuck. Any workaround for that?

claudiuconstantin commented Jun 30, 2017

@aluanhaddad, nice work, all good if you directly use rxjsin your project. What if a certain package in your project also uses the "bad" rxjs? You'll also get the error from there and until the maintainer fixes the problem you're stuck. Any workaround for that?

@Koslun

This comment has been minimized.

Show comment
Hide comment
@Koslun

Koslun Jun 30, 2017

@claudiuconstantin If another dependency directly depends on it, i.e. not just a peer dependency, you have RxJS in your project the same way you have it if you depended on it directly. So the augmentation should work just as well.

Koslun commented Jun 30, 2017

@claudiuconstantin If another dependency directly depends on it, i.e. not just a peer dependency, you have RxJS in your project the same way you have it if you depended on it directly. So the augmentation should work just as well.

@fletchsod-developer

This comment has been minimized.

Show comment
Hide comment
@fletchsod-developer

fletchsod-developer Jul 6, 2017

fyi the fix already landed on RxJS 5.4.2 (changelog), you can either close this issue or leave it open.

Bumping RxjS to v5.4.2 fixed it in both TypeScript and Angular in my case, and I no longer need this noStrictGenericChecks workaround & can safely take it out from tsconfig.json too.

fletchsod-developer commented Jul 6, 2017

fyi the fix already landed on RxJS 5.4.2 (changelog), you can either close this issue or leave it open.

Bumping RxjS to v5.4.2 fixed it in both TypeScript and Angular in my case, and I no longer need this noStrictGenericChecks workaround & can safely take it out from tsconfig.json too.

@benlesh

This comment has been minimized.

Show comment
Hide comment
@benlesh

benlesh Jul 20, 2017

@DanielRosenwasser @OliverJAsh FYI: This was resolved with the release of rxjs 5.4.2, and this issue can be closed.

All: rxjs 5.4.2 resolves this issue.

Either way, this issue can be closed as it was a bug in rxjs, not TypeScript.

benlesh commented Jul 20, 2017

@DanielRosenwasser @OliverJAsh FYI: This was resolved with the release of rxjs 5.4.2, and this issue can be closed.

All: rxjs 5.4.2 resolves this issue.

Either way, this issue can be closed as it was a bug in rxjs, not TypeScript.

@OliverJAsh OliverJAsh closed this Jul 21, 2017

AllenBW added a commit to AllenBW/manageiq-ui-service that referenced this issue Jul 26, 2017

AllenBW added a commit to AllenBW/manageiq-ui-service that referenced this issue Jul 27, 2017

@Microsoft Microsoft locked and limited conversation to collaborators Jun 14, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.