Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: re-envision Encapsulated Tasks #466

Open
machty opened this issue Jun 2, 2022 · 2 comments
Open

RFC: re-envision Encapsulated Tasks #466

machty opened this issue Jun 2, 2022 · 2 comments

Comments

@machty
Copy link
Owner

machty commented Jun 2, 2022

Placeholder RFC; TODO: add more details.

This is a design for a future API for encapsulated tasks that should be TS-friendly and support all of today's use cases for encapsulated tasks:

  • We expose TaskInstance as a class that can be subclassed
  • The same task() factory fn will accept the subclass as an arg
  • The host object will be accessible (and well-typed) as this.context
    • If you don't need access to the host obj (i.e. you are Very Encapsulated) then you can leave off the <MyComponent> type arg
class MyTaskInstance extends TaskInstance<MyComponent> {
  @tracked counter = 0;

  async perform(inc: number) {
    while(true) {
      await timeout(500);
      this.counter += inc;
      this.context.componentCounter += inc;
    }
  }
}

class MyComponent extends Component {
  @tracked componentCounter = 0;

  myTask = task({ drop:true }, MyTaskInstance);
}

// elsewhere: {{perform myTask 1}}, or this.myTask.perform(1)
@machty
Copy link
Owner Author

machty commented Aug 28, 2022

Playing around with the syntax a bit; the above could also be written as

class MyComponent extends Component {
  @tracked componentCounter = 0;

  static MyTaskInstance = class extends TaskInstance<MyComponent> {
    @tracked counter = 0;
  
    async perform(inc: number) {
      while(true) {
        await timeout(500);
        this.counter += inc;
        this.context.componentCounter += inc;
      }
    }
  }

  myTask = task({ drop:true }, MyComponent.MyTaskInstance);
}

or even

class MyComponent extends Component {
  @tracked componentCounter = 0;

  myTask = task(
    { drop: true },
    class extends TaskInstance<MyComponent> {
      @tracked counter = 0;

      async perform(inc: number) {
        while (true) {
          await timeout(500);
          this.counter += inc;
          this.context.componentCounter += inc;
        }
      }
    }
  );
}

@maxfierke
Copy link
Collaborator

maxfierke commented Aug 29, 2022

I like the proposed API a lot because it removes the need to do a bunch of the meta-programming-type stuff we do now to mixin bits of the TaskInstance API with the user's object prototype. A couple of thoughts:

  1. We'll need to be a lot more thoughtful about API surface if we allow direct sub-classing of TaskInstance. We may want to refactor some things to discourage reaching into the juicy bits. Maybe time for private fields?

  2. this.context has always felt a little icky to me. I wonder if something a bit more guarded like a getContext or getHost yieldable (or I guess, Awaitable now?) would be a better method for getting a reference to the host object. It would discourage grabbing things off of this. that aren't defined locally in the subclass and also doesn't tie the task definition to an implementation detail of TaskInstance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants