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: use "unknown" type for operators that ignore input values #6738

Merged
merged 2 commits into from Jan 11, 2022

Conversation

Wyzard256
Copy link
Contributor

@Wyzard256 Wyzard256 commented Jan 5, 2022

Description:

After upgrading an application's typescript-eslint dependency from 4.30 to 5.9, I found that the "@typescript-eslint/no-unsafe-argument" rule was flagging my use of the RxJS mapTo and ignoreElements operators, because they return functions that operate on observables of any. Since the operator functions ignore the values from the source observable, they don't need the dynamic typing that any provides for accessing the values. Using unknown instead makes it clear (to both eslint and humans) that the operator function doesn't have any expectations about the type of observable it's given.

This PR changes OperatorFunction<any, ...> to OperatorFunction<unknown, ...> in the return type of ignoreElements and the mapTo family of operators. No implementation changes are needed in the functions, since the associated values are never used anyway. This shouldn't be a breaking change: unknown is the same as any from a caller's perspective (no restrictions on what can be passed in), and OperatorFunction<unknown, ...> is assignable to OperatorFunction<any, ...> so anything that explicitly expects the latter is still OK.

These operators ignore the values produced by the source Observable, so
they don't need the dynamic typing that "any" would provide for
accessing the values.  Using "unknown" instead of "any" avoids warnings
from eslint rules like @typescript-eslint/no-unsafe-argument in
application code that uses the operators, and it's also a clearer way to
express that the operators don't know or care what the source type is.
benlesh
benlesh approved these changes Jan 7, 2022
@Wyzard256
Copy link
Contributor Author

@Wyzard256 Wyzard256 commented Jan 8, 2022

Looks like the CI build failed because it's an API change? I'm not familiar with api_guardian typical usage and it's not mentioned in the contributor guide, so I don't know whether there's an action I should take (like editing api_guard/dist/types/index.d.ts as part of this PR), or if it's something that a maintainer will handle.

(Edit: looking at other PRs, I see that they include modifications to the api_guard file for intentional API changes, so I'll do the same.)

@benlesh benlesh added 7.x 8.x labels Jan 11, 2022
@benlesh benlesh merged commit 67cb317 into ReactiveX:master Jan 11, 2022
5 checks passed
@Wyzard256 Wyzard256 deleted the unknown-source-type-operators branch Jan 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
7.x 8.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants