Setting tracked state inside a resource

[swastik]

Hi there! I have some code like this (simplified):

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { useResource } from 'ember-resources';
import { action } from '@ember/object';
import { trackedFunction } from 'ember-resources/util/function';

let loadUsers = () => {
  return new Promise((resolve) => {
    setTimeout(() => resolve([1,2,3]), 1000);
  });
}

export default class UserList extends Component {
  @tracked isLoading = false;

  list = trackedFunction(this, async () => {
    this.isLoading = true;
    await loadUsers();
  });
}

… and used in a template like this:

{{#if this.isLoading}}
  Loading
{{else}}
  {{#each this.list as |user|}}
    {{user}}
  {{/each}}
{{/if}}

This throws a you-cannot-update-state-that-you-already-read error:

---

It makes sense to me given what I understand about ember's rendering: I've read isLoading in the if and then the trackedFunction is trying to update it. Apparently this style of code does not work with tasks (e.g. trackedTask either) — it results in a similar error when using the task's isRunning state.

While the error makes sense, it also seems confusing. I can think of a few approaches to handle this, e.g. a utility like a RemoteData that can wrap loading state inside the resource itself, so that the isLoading state isn't mutated in the same cycle, only after the promise resolves, e.g. (also simplified)

class State<T> {
  @tracked value?: T;
  @tracked isLoading: boolean;
}

export function AsyncData<T>(fn: (opts: { signal: AbortSignal }) => Promise<unknown>) {
  return resource(({ on }) => {
    let state = new State<T>();
    state.isLoading = true;
    
    fn({ signal: controller.signal })
      .then((value: T) => {
        state.value = value;
      }).finally(() => {
        state.isLoading = false;
      });

    return state;
  });
}

That said, I'm wondering if this is known / if we're doing something wrong, and if there are any other possible approaches to this. Thank you! 😄

[NullVoxPopuli]

The issue is more to do with reading the tracked data before changing it (which we can see is occuring in your template -- since resources (and tracked-data) are pull-based -- meaning this.isLoading = true wouldn't happen if the resource wasn't accessed).

I have a longer write here about the problem: #340 (comment)

And a related eslint proposal:

• Add lint for protecting people from accidental infinite loops / infinite revalidation errors ember-cli/eslint-plugin-ember#1413

It's possible I need to write some lints for ember-resources, because with this pattern (and even with concurrency-tasks, which are similar):

export default class UserList extends Component {
  @tracked isLoading = false;

  list = trackedFunction(this, async () => {
    this.isLoading = true;
    await loadUsers();
  });
}

you don't want to do this. The abstraction manages its own state.
I don't think any inline resource or concurrency task should set state outside of what it defines.

So, maybe I should write the following lint rules? could probably save a bunch of folks some time:

• no-this-property-assignment-in-tracked-function
  • prevents the situation in your example (thanks for providing such a detailed issue report!!! 🎉 )
• no-this-property-assignment-in-function-resource
  • same as the above but for resource({ })
• no-this-property-assignment-in-resource-thunk
  • in the same spirit of https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-side-effects.md

[swastik]

Thank you! That all makes sense to me, and those lint rules sound great too. 😄

Curious: what do you think about that confusion for tasks? e.g. when someone writes this:

{{#if this.loadUsersTask.isRunning}}
  Loading…
{{else}}
  {{#each this.users as |user|}}
    {{user}}
  {{/each}}
{{/if}}

… and I have a component like so:

export default class UserList extends Component {
  @tracked isLoading = false;

  users = trackedTask(this, this.loadUsersTask, () => [this.args.workspaceId])

  @task loadUsersTask() {
    return fetch('/users');
  }
}

… this throws a similar error. This feels more confusing because we're not changing isRunning ourselves—it's done automatically by ember-concurrency. 🤔

[swastik]

Hm, I just realised that not accessing the task directly for isRunning, and instead using the resource works well. So that may be the way to do it. e.g.

{{#if this.users.isRunning}}
  Loading…
{{else}}
  {{#each this.users as |user|}}
    {{user}}
  {{/each}}
{{/if}}

I think I understand why that works too. 🤔