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

chore: deprecate thisArg signatures #6361

Merged
merged 4 commits into from
May 6, 2021

Conversation

cartant
Copy link
Collaborator

@cartant cartant commented May 5, 2021

Description:

This PR deprecates the thisArg signatures.

Unfortunately, the deprecation of predicates that accept a source parameter isn't possible. When the non-deprecated signatures - i.e. those with predicates that do not accept source parameters - are added, the inference breaks and calls that pass source-accepting predicates won't compile - for the dtslint tests - because the parameters are inferred as any and implicit any is disallowed. I can guess at what's happening internally - 'cause, normally, you would not have separate signatures - but the inference failure is still weird.

The removal of the source parameters will have to be conveyed in documentation instead. Or left as a breaking change surprise.

Related issue (if exists): #6143

@benlesh
Copy link
Member

benlesh commented May 5, 2021

Hmm... it would be great if this worked perfectly... but part of me thinks that it's also okay if we just document it if we can't get this working just right.

@cartant cartant mentioned this pull request May 5, 2021
40 tasks
@cartant cartant changed the title WIP: chore: deprecate thisArg and source parameters WIP: chore: deprecate thisArg signatures May 6, 2021
@cartant cartant changed the title WIP: chore: deprecate thisArg signatures chore: deprecate thisArg signatures May 6, 2021
@cartant cartant marked this pull request as ready for review May 6, 2021 07:04
@cartant cartant requested a review from benlesh May 6, 2021 07:04
@cartant cartant added the 7.x Issues and PRs for version 7.x label May 6, 2021
Copy link
Member

@benlesh benlesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Are we able to test these deprecations with dtslint?

@benlesh benlesh merged commit aa9c3d4 into ReactiveX:master May 6, 2021
@cartant cartant deleted the cartant/issue-6143 branch May 15, 2021 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x Issues and PRs for version 7.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants