NullVoxPopuli/ember-resources

Change the implementation of `trackedFunction` such that loading/error state are revealed

Closed this issue ยท 9 comments

Gonna copy @chriskrycho's https://v5.chriskrycho.com/journal/async-data-and-autotracking-in-ember-octane/
(but without the assertions for now, cause that's a breaking change).

userland API would be the same as trackedFunction is today, but in addition to .value, there would be .isPending, .isError and .error properties added

Better yet, just use and re-export ember-async-data Instead of reimplementing. ๐Ÿ˜‰

v2 addon soon?

I would happily review and merge a PR converting it! I just don't have time right now to tackle that myself!

excellent :D

I'm still trying to figure out how to properly wrangle TS + Rollup + addon-dev tho ๐Ÿ˜…

Link fof reference: https://github.com/chriskrycho/ember-async-data

Just want to confirm that this would allow me to easily test a component doing API request with useFunction:

new

{{#if this.dataset.isPending}}
    <LoadingIndicator data-test-loading-indicator />
{{else}}
    {{this.dataset.value}}
{{/if}}

existing

{{#if this.dataset.value}}
    {{this.dataset.value}}
{{else}}
    <LoadingIndicator data-test-loading-indicator />
{{/if}}

test

  test(
    'it displays a loading indicator while the dataset is loading',
    async function (assert) {
      this.server.timing = 5000; // exaggerated for observability
      await render(hbs`<DatasetInfo />`);
      assert
        .dom('[data-test-loading-indicator]')
        .exists('should render the loading indicator at component render');
    }
  );

With the current approach the test waits for the whole 5000ms before running the assertion.

It would, except i wan only considering this change for trackedFunction, as i believe it has better ergonomics

(I'm on 3.2.1)

How would you represent that there is an async operation to the user?

I'm curious if the behavior of useFunction i'm observing is what's expected. I wrote two tests, one that checks for a loading indicator and one that checks for the render of required values. The loading indicator test always fails because the assertion runs on the template only after there is a value. It's like there is a test waiter that make the assertions run only after the re-render.

It looks like the proper way of doing this is to rely on a Resource / useResource and have internal state. Would you agree?

I guess I could do this:

{{#if this.isLoading}}
  <LoadingIndicator />
{{/if}}
{{#if this.dataset.value}}
  {{this.dataset.value}}
{{/if}}
  @tracked isLoading = true;
  useFunction(() => {
    const response = await fetch(`/characters/`) ;
    this.isLoading = false;
    return response;
  })

but I have the impression that changing component tracked properties like this is not what one would want to do.

changing component tracked properties like this is not what one would want to do.

correct, this is a side effect, which can be pretty spooky beyond simple examples.

and, this is also why resources and derived state are such good patterns for encapsulating this kind of state.

How would you represent that there is an async operation to the user?

the way you were doing previously is sufficient for most cases.
Next time I find some time to work on ember-resources, I'm going to pull in @chriskrycho's library and that'll handle all the edge cases, error states, etc. (and will require another major version bump due to non-backwards compatible behavior -- which I can work around in v4, but unless a ton of people are using v4 (which I have no stats on), idk if it's worth it to absorb the API differences in a v4 minor? idk, open for discussion)).

It looks like the proper way of doing this is to rely on a Resource / useResource and have internal state. Would you agree?

yeah, most of what I like using those abstractions for is for building other resources that are nicer in actual app code.

  • Resource -- will be extended to load my specific data (or based on some convention, a kind of data, like what ember-data-resources does)
  • useResource - abstracted away behind another helper function that I usually don't use the word use in front of -- like, loadRecord or loadRecords, for example -- this pattern can provide defaults, runtime argument checking, etc

๐ŸŽ‰ This issue has been resolved in version 4.5.0 ๐ŸŽ‰

The release is available on:

Your semantic-release bot ๐Ÿ“ฆ๐Ÿš€