Proposal: If input is function, call it to use return value as data

[andrelandgraf]

Hey! I am not sure if you are currently considering feature proposals. This is just a small enhancement that I wish TrackedAsyncData would support. Happy for any reviews/suggestions/support to get this merged. However, feel free to disregard if you think this is out-of-scope! :)

Motivation

We use TrackedAsyncData (222 results in 63 files), mostly to fetch data on mount of a component:

export default class AuthorProfile extends Component {

  @service('authors') authorsService

  @cached
  get authorData() {
    return new TrackedAsyncData(this.authorsService.fetchAuthorData());
  }
}

We use a variety of patterns when the business logic becomes more complex, for example, returning different TrackedAsyncData for each conditional path, then-chaining additional logic on the initial promise, or wrapping everything in a new Promise.

1. Multiple TrackedAsyncData return values:

export default class AuthorProfile extends Component {

  @service('authors') authorsService

  @cached
  get authorData() {
    if(this.isV2Enabled) {
       return new TrackedAsyncData(this.authorsService.fetchAuthorDataV2());
    }
    return new TrackedAsyncData(this.authorsService.fetchAuthorData());
  }
}

2. Then-chaining:

export default class AuthorProfile extends Component {

  @service('authors') authorsService

  @cached
  get authorData() {
    return new TrackedAsyncData(this.authorsService.fetchAuthorData().then(async (author) => {
        return toAuthorProfile(author);
    }));
  }
}

3. Wrapping in a new Promise:

export default class AuthorProfile extends Component {

  @service('authors') authorsService

  @cached
  get authorData() {
    return new TrackedAsyncData(new Promise(async (resolve, reject) => {
      try {
        if(!this.isCommentsEnabled) {
          const author = await this.authorsService.fetchAuthorData();
          return resolve({ author });
        }
        const [author, comments] = await Promise.all([
          this.authorsService.fetchAuthorData(),
          this.authorsService.fetchAuthorComments()
         ]);
         resolve({ author, comments });
      } catch(err) {
        this.errorService.report(err);
        reject(err);
      }
    }));
  }
}

All of these work perfectly fine. However, I wish you could pass TrackedAsyncData an inline callback function directly without the need to wrap it in a new Promise or use then chaining:

export default class AuthorProfile extends Component {

  @service('authors') authorsService

  @cached
  get authorData() {
    return new TrackedAsyncData(async () => {
      try {
        if(!this.isCommentsEnabled) {
          const author = await this.authorsService.fetchAuthorData();
          return { author };
        }
        const [author, comments] = await Promise.all([
          this.authorsService.fetchAuthorData(),
          this.authorsService.fetchAuthorComments()
         ]);
         return { author, comments };
      } catch(err) {
        this.errorService.report(err);
        throw err;
      }
    }));
  }
}

Now, I have a co-located inline/nested function that can easily be moved out to be a private helper or moved to a service without needing any more refactoring. It's also super easy to read without any wrapping, then-chaining, or similar! Would love this to be a thing!

Proposed changes

Check data TrackedAsyncData constructor argument. If it is a function, call it to use the return value as the data.

This is technically a breaking change because currently a passed-in function is used as the value (this actually happened to me on accident before). I can't think of a use case where this would be the desired behavior (have a function as a value of a TrackedAsyncData object), but that behavior would technically break now. I updated the package.json version to v2 because of this, but I also think you have existing plans for a v2 update already. Happy to merge this into a v2 branch or follow any other recommendations!

Tests

I added basic tests and type tests for the changes. Please let me know if I should add more.

Considerations

I think it would be reasonable to support this case but also would understand if you think the added scope and complexity is not worth adding this. Thank you for taking a look!