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

allow accumulate to work with single or no result #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EdJoPaTo
Copy link

some debounced functions does not need to return something or return a conclusion of what happened

some debounced functions does not need to return something or return a 
conclusion of what happened
@EdJoPaTo
Copy link
Author

EdJoPaTo commented Apr 18, 2019

Is there something preventing this to be merged?

What can I do?

@bjoerge
Copy link
Owner

bjoerge commented May 5, 2019

Thank you for taking the time to make a PR for this, @EdJoPaTo. I like the idea!
The proposed implementation can however lead to quite surprising behavior in cases where the "single value" you want to return is actually an array value. As an example, consider if instead of .some you were to use .filter here:

.some(o => o > 10)

A better solution would be to add an option that explicitly tells the debounce-function to treat the wrapped function as a single-value returning function.

@EdJoPaTo
Copy link
Author

EdJoPaTo commented May 6, 2019

You are right, I did not thought of the single value response "array".

My main point for doing this was the undefined response. Debounced workers will often have no response at all. Other single values were just the superset of this.

Having a clear interface to the method is also better than "magic" changing the interface based on the return value. I think the best variant is to allow falsy like undefined returns which returns undefined to every caller or expect the full array like the current implementation.

As you suggested an extra option could be allowing single values responses the same for every caller. But im not sure if thats really needed. The idea is there, it can be implemented when it's needed I think.

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

Successfully merging this pull request may close these issues.

2 participants