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

Filtering property with another property produces surprising behavior #692

Open
asdcdow opened this issue Jun 14, 2017 · 9 comments
Open

Comments

@asdcdow
Copy link

asdcdow commented Jun 14, 2017

The current implementation of observable1.filter(observable2), where both observables are properties, gives behavior that seems more suited to event streams. It is only applied when value changes occur. It isn't applied to the current value.

I would expected this:

prop1.filter(prop2)

to be equivalent to:

prop2.flatMapLatest(value2 =>
  prop1.filter(value2)
)

instead it appears to be equivalent to:

prop2.changes().flatMapLatest(value2 =>
  prop1.changes().filter(value2)
).toProperty()

Am I wrong in this expectation? Is there some other way I should be accomplishing this?

@raimohanska
Copy link
Contributor

Can you come up with a jsfiddle, codepen or similar so that we can discuss the issue with runnable code?

@asdcdow
Copy link
Author

asdcdow commented Jun 15, 2017

Here you go: https://jsfiddle.net/gse8w7h4/

@asdcdow
Copy link
Author

asdcdow commented Aug 3, 2018

Any chance this behavior might change? It looks like it missed the 2.0 boat.

@raimohanska
Copy link
Contributor

I see what you mean but am in the opinion that it works as expected. In case of src.filter(p) it will include all values from src which occur when p holds true. This, of course, means that if p was falsy at the occurrence of a value, the value won't be included even if p changes to true before the next value is emitted by src.

I can, though, imagine your expected behavior to be useful too!

Anyways, the current implementation is useful and better aligned with the filter(f : V => boolean) version.

If you need your version, you can of course extract it as a helper function and use it. You can even add it to the Observable prototype as in Observable.prototype.filter2 = function(p) { ... }.

@asdcdow
Copy link
Author

asdcdow commented Aug 4, 2018

Isn't that entire point of properties vs event streams that they have a current value? Isn't that kind of confusing that some operations ignore the current value but some use it?

That being said, I understand that the call is yours to make. If you don't like the above behavior change, how would you feel about a pull request for a filterProperty method (or some other name) with my expected behavior?

@raimohanska
Copy link
Contributor

Hmmm. The current filter method does not ditch the current value or anything: it passes it through the same filter as any other values. It just doesn't re-send the current value in the case the filter property changes to truthy afterwards.

Can you describe an actual use case where the your described behaviour is desirable?

@asdcdow
Copy link
Author

asdcdow commented Aug 15, 2018

Sorry, I didn't mean to imply that the current value is lost altogether. I meant that when I think of operations on properties, such as:

const outputProp = someOperation(inputProp1, inputProp2, inputProp3)

I have an assumption in my head that if any of the input props change then that will be immediately reflected in the output property. It shouldn't matter what order the changes came in. The output should always reflect the current values for the input. To me that is the big difference between using a property and an event stream. The current behavior of filter seems like it is using event stream behavior for properties. Please let me know if you think I'm wrong about those assumptions.

I also feel like there is a parallel between the bacon 1 problem where people would start adding listeners to lazy properties and be confused as to where there was no current value.

Enough rambling. To answer your actual question. The case where I make use of this the most is when I'm writing a single page application and I have certain behavior that needs to be triggered on different page routes. Usually these are ajax calls. So, I will have logic that updates properties on other pages and then I will want to want to make ajax calls that automatically update the info on a particular page when the user navigates there. Does that makes sense?

@raimohanska
Copy link
Contributor

Yeah, I get it. It's just that filter in this form isn't the right tool. You can of course do

Bacon.combineAsArray(a,b).filter(([a,b]) => b).map(([a,b]) => a)

but yeah, I get it that something more handy would be in order.

@asdcdow
Copy link
Author

asdcdow commented Aug 16, 2018

Yeah, that is basically what we have settled on as the most straightforward way to do it with the current API. If you think there is something that makes sense in bacon and would make this more straightforward, I would be happy to try to put together a pull request.

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