universal-ember / reactiveweb

Reactive primitives for the web
https://reactive.nullvoxpopuli.com
MIT License
11 stars 5 forks source link

Add functionality to make debounce not return undefined initially #59

Closed BoussonKarel closed 9 months ago

BoussonKarel commented 9 months ago

Summary

I want the debounce function to intially return a value, and debounce all changes after that.

Details

I added a third optional argument to debounce debounceInitialValue which is true by default. When you set this to false, the initial value will be returned on resource creation. Otherwise, the initial value will only be returned after ms milliseconds.

I added a rendering test to make sure it works properly.

vercel[bot] commented 9 months ago

@BoussonKarel is attempting to deploy a commit to the universal-ember Team on Vercel.

A member of the Team first needs to authorize it.

BoussonKarel commented 9 months ago

If anyone could add some TypeScript magic to it so the interpreter knows that when debounceInitialValue is false, the return value will not be undefined here:

class TrackedValue<T> {
  @tracked value: T | undefined; // only undefined when debounceInitialValue is false
}
vercel[bot] commented 9 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
reactive ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 18, 2024 4:54pm
NullVoxPopuli commented 9 months ago

Any chance you could run pnpm lint:fix and commit the changes? thanks!

NullVoxPopuli commented 9 months ago

Thanks for this! All that's left is (js)docs, and helping me (and others) see the use case :sweat_smile: (or at the very least this PR serves as great documentation!)

I want the debounce function to initially return a value, and debounce all changes after that.

Normally the way to handle initial values is to have a getter that provides the logic for when to have the initial value:

class Test {
  @tracked text = 'someValue';
  @use debouncedText = debounce(100, () => this.text);

  get withDefault() {
    return this.debouncedText ?? 'initial text';
  }
}

I know in Discord, you said "what if the initial value is undefined"?

undefined ?? 'default'
// "default"
false ?? 'default'
// false
0 ?? 'default'
// 0
null ?? 'default'
// "default"

I think the question is important if you want undefined instead of your default value -- but I'd like to have an example and use case defined, ya know?

Thanks!

BoussonKarel commented 9 months ago

For example:

You have a query for people, in which the id defaults to 1.

The user can also clear the id, set it to undefined, in which case no person will be returned.

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { use } from 'ember-resources';
import { debounce } from 'reactiveweb/debounce';
import { RemoteData } from 'reactiveweb/remote-data';

class Demo extends Component {
  /**
   * By default, this returns the person with id 1
   */
  @tracked personId = '1';

  // Delay: 100ms
  // Do not debounce initial value (do not return undefined at first)
  @use debouncedId = debounce(200, () => this.personId, true);

  private personRequest = trackedFunction(this, async() => {
    if (this.debouncedId) {
      return await fetch(`https://swapi.dev/api/people/${this.debouncedId}`).then((r) => r.json);
    }

    return undefined;
  })

  /**
   * Returns the found person or undefined
   */
  get person() {
    return this.personRequest.value;
  }
}

In your example, this will never return undefined:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { use } from 'ember-resources';
import { debounce } from 'reactiveweb/debounce';
import { RemoteData } from 'reactiveweb/remote-data';

class Demo extends Component {
  /**
   * By default, this returns the person with id 1
   */
  @tracked personId;

  // Delay: 100ms
  // Do not debounce initial value (do not return undefined at first)
  @use debouncedId = debounce(200, () => this.personId, true);

  get idWithDefault() {
    return this.debouncedId ?? '1';
  }

  private personRequest = trackedFunction(this, async() => {
    if (this.idWithDefault) {
      return await fetch(`https://swapi.dev/api/people/${this.idWithDefault}`).then((r) => r.json);
    }

   // Unreachable code
    return undefined;
  })

  /**
   * Will never return undefined, because the value is always 1 or something else
   */
  get person() {
    return this.personRequest.value;
  }
}
NullVoxPopuli commented 9 months ago

can you forsee any downside with always returning the initial value and not having the third argument?

I was (re)doing some research on "debounce" in general, and every implementation assumes there is no initial value, because the function is used in response to a user action, where timing isn't so important.

It almost makes me wonder if if the behavior you want should be in a new util, throttle, perhaps?

The main thing I kind of want to avoid is "configuration" for each util, ya know? We have unlimited space to add as many utils as we want though.

BoussonKarel commented 9 months ago

Yes, there can be a downside of always returning the initial value.

For example you have a derived query property. In some resource, a network request is made based on this query.

50ms later, this query is updated because some dependency of it has updated

So there are 2 requests: @ 0ms with the initial query @ 150ms with the updated query

When query is undefined at first, there is only 1 request @ 150ms.

NullVoxPopuli commented 9 months ago

Closing in favor of https://github.com/universal-ember/reactiveweb/pull/69