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

Subject::lift has incorrect type signature #2539

Closed
hearnden opened this issue Apr 10, 2017 · 49 comments
Closed

Subject::lift has incorrect type signature #2539

hearnden opened this issue Apr 10, 2017 · 49 comments
Labels
TS Issues and PRs related purely to TypeScript issues

Comments

@hearnden
Copy link

hearnden commented Apr 10, 2017

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>

@hearnden
Copy link
Author

#2540

@kwonoj kwonoj added TS Issues and PRs related purely to TypeScript issues Next Major Version labels Apr 16, 2017
@tetsuharuohzeki
Copy link
Contributor

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.

@benlesh
Copy link
Member

benlesh commented Jun 27, 2017

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.

@benlesh
Copy link
Member

benlesh commented Jun 27, 2017

@wulfsberg
Copy link

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.

@foxel
Copy link

foxel commented Jun 28, 2017

Will this be fixed for 5.x branch?

@floodedcodeboy
Copy link

The sister issue from the TypeScript library: microsoft/TypeScript#16593

@DanielRosenwasser
Copy link
Contributor

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

@ScottRFrost
Copy link

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.

@Meligy
Copy link

Meligy commented Jun 28, 2017

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.

@xnnkmd
Copy link

xnnkmd commented Jun 29, 2017

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

@phillashworth
Copy link

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

@bobheath33435
Copy link

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

@david-driscoll
Copy link
Member

david-driscoll commented Jul 3, 2017

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]

@bmayen
Copy link

bmayen commented Jul 3, 2017

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

@MitchTalmadge
Copy link

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

@bobheath33435
Copy link

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

@bobheath33435
Copy link

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

@bobheath33435
Copy link

bobheath33435 commented Jul 3, 2017

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

@david-driscoll
Copy link
Member

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.

@devoto13
Copy link
Contributor

devoto13 commented Jul 3, 2017

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

@MitchTalmadge
Copy link

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

@bmayen
Copy link

bmayen commented Jul 3, 2017

Or, again, --noStrictGenericChecks

@hudsontavares
Copy link

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

@bmayen
Copy link

bmayen commented Jul 3, 2017

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

@hudsontavares
Copy link

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

@bobheath33435
Copy link

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

@seantanly
Copy link

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

@bobheath33435
Copy link

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

@flauc
Copy link

flauc commented Jul 3, 2017

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

@swftvsn
Copy link

swftvsn commented Jul 3, 2017

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

@mattpodwysocki
Copy link
Collaborator

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

@bobheath33435
Copy link

@mattpodwysocki Who are you?

@bobheath33435
Copy link

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

@kwonoj
Copy link
Member

kwonoj commented Jul 3, 2017

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.

@ReactiveX ReactiveX locked and limited conversation to collaborators Jul 3, 2017
@benlesh
Copy link
Member

benlesh commented Jul 3, 2017

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.

@benlesh
Copy link
Member

benlesh commented Jul 5, 2017

Closed by #2722

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
TS Issues and PRs related purely to TypeScript issues
Projects
None yet
Development

No branches or pull requests