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

fix(Subject): lift signature is now appropriate for stricter TS 2.4 c… #2722

Merged
merged 1 commit into from
Jul 5, 2017
Merged

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Jul 4, 2017

…hecks

Type safety wasn't really gauranteed with lift before because we survived for a long time with an incorrect type signature, TypeScript 2.4 introduces stricter type-checking, and all of a sudden lift on Subject was breaking builds for those that are riding the wave of the latest-and-greatest.

EDIT: Upon review, really not sure of any way this change could break anyone. So I'm going to push this through as a patch. It was basically building with an incorrect type signature in TS 2.3 and lower, so correcting it should not break anyone's builds.

@hansl
Copy link
Contributor

hansl commented Jul 4, 2017

LGTM 👍

I think it's highly unlikely, unless someone uses lift incorrectly which should have undefined behaviour.

@rxjs-bot
Copy link

rxjs-bot commented Jul 4, 2017

Messages
📖

CJS: 3161.7KB, global: 585.8KB (gzipped: 107.6KB), min: 138.0KB (gzipped: 29.1KB)

Generated by 🚫 dangerJS

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.734% when pulling c53782e on benlesh:fix-lift into 3bb6240 on ReactiveX:master.

…pt 2.4 checks

Type safety wasn't really gauranteed with lift before because we survived for a long time with an incorrect type signature, TypeScript 2.4 introduces stricter type-checking, and all of a sudden `lift` on `Subject` was breaking builds for those that are riding the wave of the latest-and-greatest.
@benlesh benlesh changed the title fix(Subject): lift signature is now appropriate for strictor TS 2.4 c… fix(Subject): lift signature is now appropriate for stricter TS 2.4 c… Jul 5, 2017
@coveralls
Copy link

coveralls commented Jul 5, 2017

Coverage Status

Coverage remained the same at 97.734% when pulling 8ed7054 on benlesh:fix-lift into 3bb6240 on ReactiveX:master.

@benlesh benlesh merged commit 9804de7 into ReactiveX:master Jul 5, 2017
@tetsuharuohzeki
Copy link
Contributor

@benlesh

I feel we can close #2539 with "we has released hotfix once for this problem. please file a new issue if you find a corner case that v5.4.2 could not caught". What to do?

@corvinrok
Copy link

#2539 was closed, it appears, but because of CoC issues not due to it being tagged as resolved. Just for linking and clarity, it might be good to have it marked as fixed in the new release (unless that is not what this fix is claiming). Thanks for the quick resolve on this!

@fletchsod-developer
Copy link

fletchsod-developer commented Jul 6, 2017

#2540 - linking reference there to here, over similiar issue.

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants