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

distinctUntilChanged should set prev value with keySelector if available even at the first time #6012

Closed
preda7or opened this issue Feb 9, 2021 · 7 comments

Comments

@preda7or
Copy link
Contributor

preda7or commented Feb 9, 2021

Bug Report

Current Behavior
distinctUntilChanged on master seem not to set the prev value correctly the first time.

Expected behavior
the prev value should be set with keySelector if available even at the first time.

Reproduction

** Please provide repro code does not involves framework, or other setups to address this bug is specific to rxjs core **
f467aa2?branch=f467aa25da69859d1540413c6c2d5a289f2c88d2&diff=split

Environment

  • Runtime: -
  • RxJS version: master branch

Possible Solution
see above

Additional context/Screenshots

@cartant
Copy link
Collaborator

cartant commented Feb 10, 2021

If this issue relates to a bug effected by the current implementation, it needs to include a code snippet that effects the bug. TBH, I don't understand what's being said here.

@preda7or
Copy link
Contributor Author

So the commit I linked does not count as code snippet?

@cartant
Copy link
Collaborator

cartant commented Feb 10, 2021

TBH, I don't understand what's being said here.

@preda7or
Copy link
Contributor Author

What I was trying to say, thinking that someone will check the linked commit too, that the refactored code of distinctUntilChanged operator looks incorrect to me:

When 'first' is true (so the first time), the 'prev' variable is set to be 'value'. In every subsequent run (when 'first' is false) 'prev' is set via a ternary operator that uses the 'keySelector' function if it is defined.
So if I have a keySelector set, the first time distinctUntilChanged operator is called the value of 'prev' will be set WITHOUT the keySelector, then in the next call of distinctUntilChanged this 'prev' value will be compared with the 'value' processed WITH the keySelector. So even if the 'value' is the same, this comparison will return false.

@cartant
Copy link
Collaborator

cartant commented Feb 10, 2021

... the refactored code of distinctUntilChanged operator looks incorrect to me.

That's why I want to see a snippet that shows it won't behave has expected - i.e. I want to see a reproduction of a bug. The code looks correct to me and I don't want to spend my time investigating something that looks correct.

@preda7or
Copy link
Contributor Author

preda7or commented Feb 10, 2021

I don't want to spend my time investigating something that looks correct

Fair enough, no one wants to work.

So I created a PR, that contains a new test case and the fixed operator. Hopefully this will help, I cannot be more explicit then this: #6013

@cartant
Copy link
Collaborator

cartant commented Feb 10, 2021

Okay, I see what you mean. I presumed you'd commented on a commit that had been made in this repo.

@cartant cartant closed this as completed Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants