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(startWith): fix incorrect types #5157

Merged
merged 1 commit into from
Jan 22, 2020
Merged

Conversation

jorroll
Copy link
Contributor

@jorroll jorroll commented Nov 22, 2019

Description:

I think (not sure) that the function definition of startWith is incorrectly typed. Because startWith has function overloads, this incorrect typing has no direct user impact. Except! this incorrect function signature is shown in the documentation for startWith (at the top), creating a source of confusion: https://rxjs-dev.firebaseapp.com/api/operators/startWith

This change can be thought of as a documentation fix, though it is also updates (fixes) the internal typing of the startWith operator function.

Related issue (if exists):

This change does not effect the user-facing function overloads, so the function's public signature remains unchanged. This incorrect typing was visible in the docs however, so this change can be thought of as a documentation change.
@cartant
Copy link
Collaborator

cartant commented Nov 22, 2019

Nope. IMO, it is correctly typed. The D refers to the type of the argument passed to the operator and the T refers to the type of the value received from the source.

@jorroll
Copy link
Contributor Author

jorroll commented Nov 22, 2019

@cartant what you said sounds correct, but I don't think that startWith currently has that function signature... ?

Currently startWith looks like:

startWith<T, D>(...array: Array<T | SchedulerLike>)

But shouldn't it look like?

startWith<T, D>(...array: Array<D | SchedulerLike>)

After all,

D refers to the type of the argument passed to the operator

@jorroll
Copy link
Contributor Author

jorroll commented Nov 22, 2019

As a point of comparison, the type signature of the startWith function itself appears to be incompatible with its second function overload.

i.e. this is the first function overload:

function startWith<T, D>(v1: D, scheduler: SchedulerLike): OperatorFunction<T, T | D>;

and this is the type signature of the function:

function startWith<T, D>(...array: Array<T | SchedulerLike>): OperatorFunction<T, T | D>

@cartant
Copy link
Collaborator

cartant commented Nov 22, 2019

Yeah. You're right. I read the diff backwards.

@benlesh benlesh merged commit faaae75 into ReactiveX:master Jan 22, 2020
kwonoj pushed a commit to kwonoj/rxjs that referenced this pull request Feb 5, 2020
This change does not effect the user-facing function overloads, so the function's public signature remains unchanged. This incorrect typing was visible in the docs however, so this change can be thought of as a documentation change.
martinsik pushed a commit to martinsik/rxjs that referenced this pull request Feb 15, 2020
This change does not effect the user-facing function overloads, so the function's public signature remains unchanged. This incorrect typing was visible in the docs however, so this change can be thought of as a documentation change.
@lock lock bot locked as resolved and limited conversation to collaborators Feb 22, 2020
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.

3 participants