rxjs: Subject::lift has incorrect type signature

@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.

@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.

Will this be fixed for 5.x branch?

“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 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.

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 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.

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!

@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.

@david-driscoll Who are you?

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.

@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.

@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.

@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.

@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.

@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.

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

@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.

@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.

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 😄). And given that the signature is actually wrong currently, we’re not losing anything with any.

Real developers fix the build in 5 mins, say thank you to the awesome team here and continue their lives… 😄 just sayin’

“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!

Peace guys! And beer 😃

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.

@bmayen Not sure why, but I got the lift type error again after running npm install --save typescript@2.3.2, 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.

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

@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.

@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.

@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.

@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.

@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.

@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<any> to solve the problem in a non-breaking way in the short term.

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

Or, again, --noStrictGenericChecks

@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.

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

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. 😄

@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.