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

Nitpick: Shouldn't Rx.pipe with no arguments return identity instead of noop? #4933

Closed
leewz opened this issue Jul 26, 2019 · 3 comments · Fixed by #4936
Closed

Nitpick: Shouldn't Rx.pipe with no arguments return identity instead of noop? #4933

leewz opened this issue Jul 26, 2019 · 3 comments · Fixed by #4936

Comments

@leewz
Copy link
Contributor

leewz commented Jul 26, 2019

https://github.com/ReactiveX/rxjs/blob/6.5.2/src/internal/util/pipe.ts#L25

if (!fns) {
    return noop as UnaryFunction<any, any>;
}

The composition of zero things should be the identity under composition, which is Rx.identity, not Rx.noop. That allows pipe(pipe(), pipe(f)) to be equivalent to pipe([].concat([], [f]), which is equivalent to f.

I'm new to RxJS, so I don't know if people rely on this behavior.

@cartant
Copy link
Collaborator

cartant commented Jul 26, 2019

Yes, you're right. It should be identity, IMO, too.

Note that the pipe method returns this - the source observable - when called with no arguments:

if (operations.length === 0) {
return this as any;
}

@cartant
Copy link
Collaborator

cartant commented Jul 26, 2019

Would you be able to open a PR with the fix?

@leewz
Copy link
Contributor Author

leewz commented Jul 27, 2019

Actually, the conditional in my issue is not a check for the empty array, but for no array at all.

There is no test for pipeFromArray(undefined) = noop. The function is marked as @internal. The conditional is actually unreachable from client code.

@lock lock bot locked as resolved and limited conversation to collaborators May 5, 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 a pull request may close this issue.

2 participants