Debounce about vest HOT 9 OPEN

Hi @vonagam, thank you for this request 👋
I appreciate the time you have taken to make a suggestion to improve vest 🙏

I definitely see the value in debouncing async requests, and that's something that I've seen being asked for multiple times.

I am, though, not fully certain that debouncing should be something that's provided by Vest, but rather can be done in the userland (or consumer-land, rather), and there are several main reasons for that:

1. Debouncing can (and often should) be done on two different instances:
   A. On keystroke - whenever a user types within a field
   B. On Async server call.
   While Vest could obviously handle both (one on the suite entry, and one on the async field), I think it might more complexity and another level of unexpectedness to how Vest operates.

2. Debouncing is not the only volume control function we may use. Along with Debouncing, people often ask for Throttling, which is its more primitive sibling. Adding both will significantly increase the cognitive load of the library.

3. As you've experienced yourself, turning the last argument to an options object makes it more verbose, and change reduce some of the simplicity introduced by Vest. From having the optional fourth argument reserved for reordering only, we need to now use it in multiple scenarios.

4. One last thing, while useful, debounce might not be used by most users of the library. Adding it as an optional to the test function, will make it so that the debouncing logic and capabilities will be added to the core of the library and take up (even if just a little) space and increase the bundle size.

5. Technically, with debounce, the function needs to be wrapped with debounce in advance, and not written inline, since if it was written in-line, every run would create a new instance of it. Handling this within Vest is possible by caching the first (or last) version of the test function, but this, too, requires some thinking on how it should be done so it does not cause unexpe

All these do not mean I do not see the value in debouncing, but rather, I believe debouncing should be introduced in a different way. A suggestion would be to introduce debouncing as a built-in vest utility, like classnames and parser, so that you could import it separately, something like:

import {test, debounce, enforce} from 'vest';

// ...

test("username", "username is already taken", testUserAlreadyTakenDebounced(data));
// ...


const testUserAlreadyTakenDebounced = debounce(function testUserAlreadyTaken(data) {/*....*/})

What do you think?

from vest.

1. ... I think it might more complexity and another level of unexpectedness to how Vest operates.

New functionality is more complexity, understandable, but I don't see how it can increase unexpectedness.

2. Debouncing is not the only volume control function we may use. Along with Debouncing, people often ask for Throttling, which is its more primitive sibling. Adding both will significantly increase the cognitive load of the library.

Usage of throttling does not seem proper for validations to me. What happens when a new value arrives and a test call is discarded because of throttling: Does an error for a previous value stays in place even though new value is valid (and other way around old right -> new wrong)? Looks like a bad idea to me... (Also maybe we are talking about different implementations, because I wouldn't call throttling more primitive compared to debounce, they are almost identical, except maybe that debounce does not need to store a result of previous invocation.)

3. As you've experienced yourself, turning the last argument to an options object makes it more verbose, and change reduce some of the simplicity introduced by Vest. From having the optional fourth argument reserved for reordering only, we need to now use it in multiple scenarios.

Well, current way with test.memo works only if you have really small number of modificators that cannot overlap. Since for some cases (like username) one might want to use both memo and debounce there is a need for a way to provide both options. I also assume that with time you might get more functionality like that, so makes sense to think how it should be implemented. Agree that options object does increase verbosity, but it is not expected to be used that often.

4. One last thing, while useful, debounce might not be used by most users of the library.

Do agree, but I don't think that debounce is very different from memo. They exist to cover the same cases and debounce is actually easier to implement - memo needs to keep results, keep inputs of previous invocations and check those inputs on every call and have an eviction policy (currently it is hard coded to 10 elements max, perhaps if there were options it could have been configured) while debounce only need to store one integer and do setTimeout/clearTimeout with it. So my question about that - do you consider memo and debounce to be on equal footing and resulting solutions applicable to both of them or do you think that memo is more deserving to be part of the core compared to debounce and if yes, why?

5. Technically, with debounce, the function needs to be wrapped with debounce in advance, and not written inline, since if it was written in-line, every run would create a new instance of it. Handling this within Vest is possible by caching the first (or last) version of the test function, but this, too, requires some thinking on how it should be done so it does not cause unexpe.. (text is missing after that)

If it is a part of a test (like memo is) then it is the same. Only a timer id for last invocation needs to be stored.

A suggestion would be to introduce debouncing as a built-in vest utility, like classnames and parser, so that you could import it separately

Seems like a good idea if it is possible to do so.

But the shown example has problems - a) it is more verbose than options variant since you would need to import an additional thing, create/name/wrap a test function outside of a suite and then use it in a test, b) debouncing state (timer id) is stored inside the function itself, not suite, so the function cannot be used in multiple suites (unexpected part), c) it will not work with test.memo since memo will save a promise which will never resolve if debouncing happens.

If we go with decoupling option then this is what I would ideally want to work:

import {test, memo, debounce} from 'vest';

test("username", "username is already taken", debounce(memo([data.username], () => {
  // ...
})));

Three "wishes" here:

1. If debounce is in utils, so is memo.
2. The state is stored in the test context, so is it possible to move a test function definition out of a suite for small performance gain but it is not required.
3. Debounce should be able to work with memo.

from vest.