Rxjs: Subject::lift has incorrect type signature

Created on 10 Apr 2017  路  66Comments  路  Source: ReactiveX/rxjs

RxJS version: 5.3.0+

Additional information:

lift is defined on Observable<T> as:

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

Subject<T> defines lift with the signature:

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

The return type should probably be Observable<R> rather than Observable<T>

TS types

Most helpful comment

@benlesh @kwonoj @jayphelps

I think (propose) we should release this fix as the next patch version because this was our historical mistake and more intelligent tsc just found its bug. If we afraid a breaking change, we should checks our codebase on both of tsc 2.0.x ~ 2.4.x.

Of course, however, I also understand the opinion that this fix should be release as the next major version. So I propose alternatively to release this fix as "6.0" and we don't include any other changes to "6.0". And we postpone to release the current next branch as 7.0 or later. By this way, 1) we can only fix this problem without any other breaking change which may cause a hard work to upgrade rxjs. 2) we'll obey semver convention.

__I think this might fix this problem which our users are facing actually now and it's pretty important things.__

All 66 comments

I confirm this problem affects to the environment using TypeScript _2.4.1_ as stable. (It's no problem with _2.4.0_ as beta version).

And we can workaround this problem with --skipLibCheck tsc option. However, its option is not listed by tsc --help now. I'm not sure about the status and the future for --skipLibCheck.

There are two solutions I can see:

  1. Use v 6 alpha
  2. We can type the return value as Obsevable<any> to solve the problem in a non-breaking way in the short term.

The any hack sounds very good to me. I'd love to move to TypeScript 2.4 at first opportunity, given its stronger type inference (as just evidenced :smile:).
And given that the signature is actually wrong currently, we're not losing anything with any.

Will this be fixed for 5.x branch?

The sister issue from the TypeScript library: https://github.com/Microsoft/TypeScript/issues/16593

You can get around this using the --noStrictGenericChecks flag in TypeScript 2.4.1.

There's some confusion here with versions impacted. The current 5.x version of rxjs works fine with TypeScript 2.4.0, but not 2.4.1.

2.4.0 acts like 2.3 sort of, which would explain why it works.

For example, there's a lint bug where sometimes 2.3 and tslint show false 'All imports are unused.` message. This is only fixed in 2.4.1, and there's no mention in TypeScript releases for .0 or .1 version. I suppose 2.4.0 was a broken thing they quickly added the new version 2.4.1.

It's silly they are not respecting semantic versioning. But I understand they use the flags strategy instead, which might be more practical for their case.

For now we have a few workarounds:

  • Upgrading to v6, but it's not clear what this might break, couldn't find docs about it

  • Setting noStrictGenericChecks to false as mentioned earlier, although that feature the flag turns off was the main reason I went to Typescript 2.4, because it showed some bugs in some of Angular tests mock classes.

  • Setting skipLibCheck to true, which looks very promising.

@benlesh Yes please to the short term solution. With this angular and other projects that are blocked by this use typescript 2.4 (v 6 alpha is not really an option for most).

We can type the return value as Obsevable to solve the problem in a non-breaking way in the short term.

@xnnkmd projects shouldn't be blocked - to get around this, use the --noStrictGenericChecks flag.

My solution so far (Angular 2 application) is to set skipLibCheck in tsconfig

The only drawback here is that I'm now forced to use asObservable() when I need to assign Subject to Observable variable or call some not subject specific method. E.g. this._transactions.asObservable().bufferTime(1000).subscribe(...)

At least this allowed me to get all the benefits of TS 2.4

@DanielRosenwasser With --noStrictGenericChecks I lose must of the benefits of the TS upgrade, so I don't like this solution. The undocumented skipLibCheck option is better but is also problematic. Much better with the small rxjs fix that @benlesh suggested.

@DanielRosenwasser BTW: Typescript is the best and it is great that TS has type check improvements that find new bugs in libraries like this.... It would however be appreciated if you provide info about known issues with major 3rd party libs like angular and rxjs in your new version announcements.

@xnnkmd this is a rxjs bug (one that's already fixed in the dev version), not a Typescript bug. Typescript dev team shouldn't be responsible for testing every 3rd party lib for compatibility with each release.

Running an Angular app and upgrading to rxjs 6.0.0-alpha.0 causes all sorts of dependency errors. looks like staying on typescript 2.4.0 is the ideal solution for the time being.

So I understand that the rxjs 6 has a fix applied already.

But will a fix be applied to rxjs 5 any time soon?

@benlesh @kwonoj @jayphelps

I think (propose) we should release this fix as the next patch version because this was our historical mistake and more intelligent tsc just found its bug. If we afraid a breaking change, we should checks our codebase on both of tsc 2.0.x ~ 2.4.x.

Of course, however, I also understand the opinion that this fix should be release as the next major version. So I propose alternatively to release this fix as "6.0" and we don't include any other changes to "6.0". And we postpone to release the current next branch as 7.0 or later. By this way, 1) we can only fix this problem without any other breaking change which may cause a hard work to upgrade rxjs. 2) we'll obey semver convention.

__I think this might fix this problem which our users are facing actually now and it's pretty important things.__

This is the reason that you have compilers: to avoid run time problems. Here we have code that has made it all the way to the end user, and the does not even compile! Do the people who write their code test their code outside their own little development environment? This is a pretty bad vulnerability to anything that uses these packages. Hundreds, perhaps thousands, of developers and users are all serialized on this problem.

@bobheath33435 I'd say compilers are more for avoiding compile time errors, not run time. A logic error is a run time error, and the compiler isn't likely to warn you about that. Besides that point, the TypeScript developers aren't responsible for this problem. rxjs just needs an update to be compatible with the new TypeScript version. The current solution? Don't immediately upgrade to the latest TypeScript (or other library) versions on your production code and assume everything will be fine. This is what a development environment and the community is for; you upgrade, find something broken, and make others aware of the problem -- then it gets fixed and everyone is happy.

@MitchTalmadge Without a compiler you don't have compile time errors. Compilers allow us to do type checking, syntax checking etc. Without that compiler, those nasty bugs rear themselves at run time, which is more difficult and time consuming to debug, thus the creation of compilers like TypeScript for instance. When there was no compile of javascript code, that bug would have to be discovered at run time. BTW, I have a solution. I backtracked to my previous node_modules directory. I guess when the people that checked the code in get back from their July 4 vacation, we can get back to developing Angular code.

Well, whatever it is, it should be fixed. It's blocking.

@alvipeo Yeah, checking in some code with compile errors is not very cool, and then letting it sit for days. Some controls need to be somewhere to prevent this kind of behavior.

@bobheath33435 Nobody has checked in non-compiling code. You have updated your TypeScript compiler to a new version with a stricter check.
-Yes, it is an error, and it would be nice to get it fixed, but it has always been there, and has compiled and gone unnoticed up until a couple of days ago when the new TypeScript compiler got smart enough to spot it.

@wulfsberg I haven't updated the typescript compiler. I went through various package.json files trying to find where the compiler was introduced, but I couldn't find where the new compiler was referenced. I am pretty busy trying to wrap up a project, and I really don't have time to debug this code. So I just backtracked node_modules, and learned a valuable lesson: always keep your last copy of node_modules. And don't do any angular updates until you have checked it out on a trial directory.

Peace guys! And beer :)

@alvipeo I'm at peace. I don't like to revisit problems. So when I encounter a problem, I try to understand where I went wrong, and I try to avoid that behavior next time.

Fair enough.

Having a compiler (or transpiler for TypeScript) doesn't guarantee there won't be run-time errors.

Back in 1975, when COBOL 1968 transitioned to COBOL 1974, all of our existing code clean compiled, but a subtle change in how READ statements in the PROCEDURE DIVISION treated the record buffer defined in the DATA DIVISION caused numerous unexpected run-time errors.

After several days of moving record structures from the DATA DIVISION to WORKING STORAGE SECTION, changing READ to READ INTO, the problem was fixed.

My point is this: as long as human beings create computer languages, these kinds of issues will come up from time to time. Thank goodness we have development environments to try things out in first. It's not like the bad old days when we did not have separate boxes of punched cards, one for development and one for production. :-)

@fmorriso Of course compilers have not become so sophisticated that they can remove all run time errors, but they will protect against many runtime errors, thus theoretically reducing development time.

"Do the people who write their code test their code outside their own little development environment?"
"I guess when the people that checked the code in get back from their July 4 vacation, we can get back to developing Angular code."
"Some controls need to be somewhere to prevent this kind of behavior."

Ultimately, these kinds of comments attacking the team are simply not constructive @bobheath33435. This is an open source project. You're not paying for it and nobody owes you anything. The expectation is that the community participates in improving the product and we all benefit. You say that you don't have time to debug this code, yet you're upset that others haven't already fixed it for you? RxJS happens to be a very complex and very high quality repository with a proven track record. Unfortunately, bugs still come up from time to time. The community has already pointed out very simple workarounds and there's already a solid game plan to release the fix. Further attacking the team does not contribute anything to the process.

Also, you seem to be missing the critical point here that the TS compiler did not catch this beforehand because its checks weren't strict enough. This is an old bug which was just caught by the most recent TS release. So, to your point that this should have been caught by a compiler... it was.

@bmayen My comments were and are intended to be constructive. I understand how inexperienced developers like yourself may not understand that promoting incompatible code has far reaching consequences, especially right before a long holiday from which you are not going to return until the following week. Experienced developers don't make those mistakes, and when inexperienced developers make that kind of mistake, experienced developers should try to educate the inexperienced developers of their mistakes, not for retribution, but for education and community training.

Again, some inexperienced developers like yourself believe that since there is no license fee for the software, engaging in unprofessional development practices is OK. It is not OK. While there are not licensing fees, there are costs involved. Sometimes the costs are so high, that many developers forgo their July 4 holiday to complete their project, only to have that project delayed by the uninformed bad practices of inexperienced programmers working on code unrelated to the developer's code.

Been in the industry for 20+ years, but roger that. Hopefully these inexperienced RxJS devs have learned something from you today as well and will finally get their acts together!

Compiler is a good thing to have, it made our life easier & lead to fewer runtime errors - especially when there are 1,000 TypeScript files we have at work. Nobody like spending lot of time hunting down runtime errors. :-) Just saying. I'm happy TypeScript is evolving, improving and catching more errors. That is no different than C, C++ in our old days & seeing it evolving. :-D

@fletchsod-developer I agree. I think it's actually very cool that TypeScript is getting smarter and found a problem that had gone under the radar for quite some time in a very popular library. This is real world proof that TypeScript is getting better.

@bobheath33435 those comments aren't really helpful... in fact they are the reverse. I'll be sure to make sure we have an up to date Code of Conduct, but those comments are frankly not beneficial to this community, so I would please ask that you refrain from making any more of them.

You don't pay our wages, we don't get wages for this project. If you would like to purchase a support plan I'm sure @benlesh, @kwonoj or myself would be glad to help you for an appropriate fee. If your project is so critical that you cannot wait for this to be fixed, there is one simple way around it, and that is to just stick to TypeScript 2.3, it's not hard, it's not rocket surgery, it really isn't.

@bmayen If you have been in the industry for 20 years, I would have expected more from you. I would have expected you to have a better understanding of community development. I would have expected you not to put anybody through the claim used by unsophisticated developers that this software is "free" as being justification for this kind of problem. This technology is NOT free. Developers are embracing this technology at great expense. For those of you that buy into @bmayen's lack of understanding in making the argument that this is "free," thousands of developers are investing their time and careers into this technology. If the technology fails, all of us lose. If the condition of having a build that will not even compile occurs frequently, all of the development community is deadlocked; the product will fail. Developers will embrace competitive technologies.

@bmayen, if you have 20 years of development experience, I would expect you to understand this. I would expect you to work towards improving the development process instead of mocking those that are trying to improve the development project.

Back on topic however...

The original plan to mark as a breaking change may get adjusted (we as a team have not determined that) however, we currently have the long weekend, so it won't happen to later in the week. When we originally marked it as breaking, it was breaking simply because anyone that depended on that behavior in the past, would have been broken by the change. If they had inherited from Subject<T> and overridden lift they could have upgraded _RxJS_ and their code would have broken. While this is an edge case, this is still a breaking change.

This change in the TypeScript compiler is technically a breaking change, and if you follow semver should have gone with TS 3.0, however unfortunately the TypeScript team doesn't strictly follow semver, but that isn't the conversation here. You updated an outside dependency, that has caused a breaking change. This is also a dependency that we haven't yet taken, in fact we have yet taken TypeScript 2.1+ due to back compatibility reasons.

Keep in mind, there are of ways around this issue that have been outlined before:

  • Enable --noStrictGenericChecks until a new version comes out.
  • Stick with currently TypeScript 2.3 or lower.

@david-driscoll Who are you?

@bobheath33435 please stop with the unhelpful and antagonistic comments, you aren't contributing anything useful to this issue. Maybe your time would be best spent on the project you are busily trying to wrap up.

@phillashworth My intention is constructive. My best customers are customers that let me know when I do something wrong. Without that feedback, sometimes I am unaware of my mistakes. Some customers just walk away, denying me the opportunity to fix problems in my products and services.

There is a certain protocol that is used by successful projects involving many developers. That protocol makes a team much more productive in their mission of developing a functional product.

but bob, bob, we very aware of the issue, we've had at least a dozen duplicate issues at this point.

We are well aware of the problem.
We are well aware of the fix.

We do not need anyone coming in and say we're inexperienced, and that experienced devleopers don't make mistakes. I can happily tell you everyone makes a mistake, it's nothing to be ashamed of.

In your particular case, it isn't even _our_ fault that some random library you're using has typescript listed as a dependencies in it's package.json. typescript should only ever be a devDependencis. That isn't anything we did, or that we can even fix.

The fix for you? npm install --save [email protected]

"I would have expected more from you". Very sorry to disappoint you @bobheath33435. Didn't realize you were so invested in my personal growth. I'll try harder for you in the future.

In return I'll offer you this bit of advice in response to "My intention is constructive". There's a huge difference between your intention and what you actually conveyed. Your intention, whatever that may be, is completely overshadowed by the aggressive, egotistical comments you wrap it in and the fact that you're not even aware that you're repeatedly missing the point despite people calmly trying to explain it to you.

Good luck!

@bobheath33435 Dude, just relax. You're getting so worked up over something that isn't really a big deal. This is a very very unique situation in which an error went undetected by the compiler for some time (really without any apparent harm) and is just now being detected after TypeScript was updated; this is a __good thing.__ It proves that TypeScript is evolving and learning. You keep saying that the compiler should prevent these types of errors, but you've entirely missed the point which is that now it's catching more errors than ever. It's doing exactly what you want it to do! Yet all you've done thus far is bash on the team and insult people as you call for better protocols in a situation where better protocols would not have prevented it... while everyone else is patiently and respectfully working towards a solution. Yes, people are taking their holidays, and they should. A very simple workaround already exists, so lay off and let people be with their families. This will be resolved. Just have patience and please, quit with the aggressive comments.

@bmayen OK you win. I will shut up. I was unaware of being egotistical, but with your 20 years of experience, I guess that you would know.

@MitchTalmadge I am not worked up. However, I do get the impression that the community is piling on. I want this project to be successful, but I must admit, my enthusiasm is waning. There is a process that prevents this kind of condition, and if it is employed, these kind of problems are reduced in frequency. When the frequency of these types of problems is reduced, the whole community benefits. And the likelihood of this product being successful is enhanced.

@david-driscoll As I mentioned earlier, I looked through the entire project looking for the package.json file with the offending typescript spec. As I mentioned earlier, I found a work-around by restoring the previous node_modules directory. My package.json file in my directory tree remains unchanged. My package.json file does NOT have a reference to the incompatible typescript spec. Perhaps the offending typescript spec is in the package.json file in one of the node packages somewhere in the node_modules directory, but there are just too many packages for me to look into every package.json file.

I tried to diagnose the problem, but the comments here led me to believe that the problem had already been diagnosed and fixed. I am awaiting the fix to be built into the new build, which will enable me and others to move on with our development of an angular app.

You could run a script to grep or search through all the package.json files (grep/powershell/etc). If you add that one line to your package.json, it should override the lower package.json files, and pin you to TypeScript 2.3.

@bobheath33435 npm ls typescript will show you all instances of typescript package with dependencies, that brought them into the project.

@bobheath33435 do you have a caret ^ in front of the TypeScript version number in your package.json? I think that fits the behavior you mention.

Or, again, --noStrictGenericChecks

@david-driscoll npm install --save [email protected] will fix the running instance but may cause some problems if an automated tool deploys the code, downloading the dependencies again.

The safer way still being to add noStrictGenericChecks to the tsconfig.json file.

CI would still respect package.json. Unless I'm missing a point?

@bmayen Not sure why, but I got the lift type error again after running npm install --save [email protected], deleting the node_modules folder and attempting to just npm install. In other hand, the noStrictGenericChecks endured to this test.

I hope it may be some project-specific or environment-specific problem (unfortunately I am on the midst of a timed task, so I cannot check further for now), my comment was more of a heads-up rather than an assertion.

@david-driscoll I went through a bunch of these exercises already, but I tried again. Uninstalling the angular cli, reinstalling, and doing a npm install of typescript 2.3.2 does not create a working environment. I tried. I wish it would work, but it doesn't work. If you want real developers working on angular apps, this process needs to be cleaner.

This following works for me. Just add augmentations.ts into the project.

// 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>
  }
}

Ref: https://stackoverflow.com/a/44813884

@david-driscoll I resorted to my previous work-around by reinstalling my project from github and copying the previous copy of the node_modules directory. Let me know if you want me to try something else out. If you want my help, I would be happy to test out the solution before you open the new build to the general public.

@seantanly After implementing the fix my build started compiling with out errors. However once I navigate to the page I get the error. Do you maybe have an idea why this would be happening?
Thanks 馃憤

Real developers fix the build in 5 mins, say thank you to the awesome team here and continue their lives.. :D just sayin'

@david-driscoll Who are you

@bobheath33435 you have been warned, please keep it civil here or you will be banned from this repository as this hostility is in violation of our CoC.

@mattpodwysocki Who are you?

@mattpodwysocki Can you please identify the passages that you deem to be hostile?

OK, this is enough.

I usually do not try to interfere directions of discussions in issue, but recent discussion in this particular issue is far close to productivity that we do not discuss about actual issues anymore.

Summarizing this issue itself

  1. This is caused by our miswritten type defnition
  2. But only caught by latest compiler, that we didn't expect minor semver bump up could cause this, reason it was under long-pole discussion to next major.
  3. We're trying to sort out this, but since it's US holiday core members are not fully managing issues.
  4. Several workaround are already suffeciently explained in this issues.

I'm locking this issue, and will lock any further dupe issues as well. Also as same as other core members, I'm seeing unmanner which possibly violates our code of conduct and it'll be discussed separately.

Just a quick note for everyone on what happened here, @bobheath33435 has been banned from this repository for repeatedly insulting and personally attacking another community number.

It's understandable that people can be frustrated when there is a bug with a library or an incompatibility with typescript 2.4. Being upset about that or even complaining about it is totally fine. However personally attacking other members of our community is prohibited by our code of conduct. Upon being warned, he seemed it to get belligerent with @mattpodwysocki, who for the record is the creator of RxJS. That was enough for me.

My apologies to others involved in this thread. I hope you do not lose taste for the RxJS community.

Happy Fourth of July weekend everybody.

Closed by #2722

Was this page helpful?
0 / 5 - 0 ratings

Related issues

OliverJAsh picture OliverJAsh  路  3Comments

cartant picture cartant  路  3Comments

dooreelko picture dooreelko  路  3Comments

Zzzen picture Zzzen  路  3Comments

giovannicandido picture giovannicandido  路  4Comments