(v2 prep): Try out passing the previous instance of a Resource to the next Resource, getting rid of the need for lifecycle hooks

[NullVoxPopuli]

This makes Resources function more like functions and hopefully will lead to more derived code, rather than imperative lifecycle-based code

[scottmessinger]

This new approach seems conceptually simpler, but I'm concerned about how it would handled use cases that v1 handles nicely now. These concerns might be unfounded, so I wanted to describe them here in case you can shed light on them!

In our app, one of the places we're using Resources is to handle query fetching. The resource receives the name of the query and the offset to fetch at. The first time the component is rendered, the offset is likely 0 and the resource fetches the first page of results. When user hits "next" or scrolls to the bottom of the component, we update the offset. The resource update method is fired, requests more results, and appends them to the resource's value.

With this implementation, where would we store the state that we need to hold on to between argument changes? Would this use case still be covered in this new model where instances are torn down on each arg change?

Another example: we're using the resource to wrap a state chart (using x-state). When args change, the update function calls back to an update function which will advance the state chart. Because the statechart holds state, we need it to be persistent between arg updates for it to be useful.

With this implementation, where should we store the state chart?

Let me know if any of this doesn't make sense -- I didn't feel my examples were as lucid as I wanted.

[josemarluedke]

@scottmessinger I think your use case makes sense. The resource should make it easy for the author to decide the best way to update when args change. @NullVoxPopuli What do you think? What was your original motivation for this change?

If this was only to allow for "function like", maybe we can explore what a function-based resource would look like, if even possible.

[NullVoxPopuli]

, where would we store the state that we need to hold on to between argument changes?

Possibly a WeakMap? I'll provide an example after I read through all your questions :D

Would this use case still be covered in this new model where instances are torn down on each arg change?

I think so, but we'll need to work through some examples to be sure

where should we store the state chart?

example time! (which is great, because I think I want to swap out ember-concurrency for a micro statechart implementation in an addon at work)

const STATE = new WeakMap<MyResource, Interpreter>();

class MyResource extends Resource {
    constructor(owner, args, previousInstance) {
      super(owner, args, previousInstance);

      if (STATE.has(previousInstance)) {
          let interpreter = STATE.get(previousInstance);

          interpreter.send('ARGS_UPDATE', args); // or however you want to handle updating args          
          STATE.set(this, interpreter);
          // the previousInstance will be destroyed, and the WeakMap will lose the reference
          return;
      }  

      STATE.set(this, = new Interpreter(stateMachine, /* ... */));
    }
}

but, where I think this falls apart is destruction -- how do we teardown the statechart interpreter? Normally, you'd:

registerDestructor(this, () => {
  interpreter.teardown();
});

But we can never do this, because we need the interpreter to be available for when the next Resource / update is constructed. I may be missing something, but it does seem like a drawback. In React, this'd be handled with effects and a useRef... but.. those are also gross. So given this constraint, idk if we actually do want to go through with this constructor-only design. 🤔

[NullVoxPopuli]

If this was only to allow for "function like", maybe we can explore what a function-based resource would look like, if even possible.

I do like this idea, and maybe it could be an alternative to the class-based API.

We already have class and function variants for:

• Helper
• Modifier

• Components (sorta, template-only are like functions~ish)

  So to me, it'd make sense that we have a function API for resources.

  Maybe:

  
  @tracked a = 1;
  @tracked b = 2;

@use added = resourceFrom(() => { return this.a + this.b; });

 ```js
 console.log(foo.added); // 3

maybe the type for resourceFrom would be:

 function resourceFrom<Return>(previousValue?: Return): Return;

but, this is almost just a helper 🤔

[pzuraq]

@scottmessinger does using the state of the previous resource work?

class MyResource extends Resource {
    constructor(owner, args, prev) {
      super(owner, args, prev);

      this.results = prev?.results;

      fetch(`${this.args.url}?offset=${this.args.offset}`).then(r => r.json()).then((results) => {
        this.results = this.results.concat(results);
      });
    }
}

I think pretty much any time you need the previous state, you should be able to read it from the previous instance and put it on the next one directly. No need to weakmap or anything that fancy.

I do think that destruction is something to consider. It may be the case that you want to avoid destroying the previous instance when creating a new one, and only want to really fully destroy when the resource is being cleaned up and no new instances are being created. Maybe destroying the previous instance should be the responsibility of the next instance, so the user maintains control.

[scottmessinger]

@pzuraq re: instances. That could definitely work! It doesn't feel it's as intuitive as the update method, but I figure it might be better for other reasons.

As for destruction:

It may be the case that you want to avoid destroying the previous instance when creating a new one, and only want to really fully destroy when the resource is being cleaned up and no new instances are being created

This sounds like it would be easy to mess up and prone to memory leaks. Obviously, you're expressing a proposal and the code might negate my worry. Also, maybe my imagination is failing me, but I can't think of the advantages that would come with being able to avoid destroying previous instances.

On the related topic of teardown: In our app, we've long wanted to know when we could tell the server to cancel the real time subscription for a model, but we never had a way of tracking when all the components were done with the model. We've used PromiseProxy's which didn't let us know when they were destroyed (or at least we never figured it out). We're excited to use the teardown hook to implement tracking where we tell the store we're using the model on setup and when we're done on teardown. The store would increment/decrement a counter for the model and release the subscription when no resources were using it. Since it looks like the teardown hook is going away, could we achieve something similar in the constructor of the next instance? If so, how would we handle teardown when the resource is finally destroyed as a new constructor isn't called?

FWIW, I'm a bit foggy on the advantage of this new model. It sounds like @NullVoxPopuli mentioned something about the goals in the PR body:

more derived code, rather than imperative lifecycle-based code

First, I don't fully know what "derived code" is -- I'm familiar with declarative vs imperative, so there might be a body of thought on why derived is better than imperative I'm not familiar with.

Second, for me, the current imperative based code of setup, teardown, and update hooks feels really nice. I'm declaring what I need in the component (with @use) and I'm implementing what I need in the resource (with some imperative code). That split feels right to me -- it concentrates the imperative code in one place that's easy to change and understand and let's me use a declarative syntax (@use) everywhere else.

This new change without lifecycle hooks feels different but not better. Obvious, this is an early PR, so the end result could be amazing, totally worth it, and I just don't have the vision to see it! I don't want my misunderstanding to be seen as opposition -- just some initial skepticism having written and used a few resources with the current API.

[pzuraq]

This change in direction has primarily come out of conversations I've been having with @krisselden and @wycats. The issue that we've seen is that the current model can cause some common mis-modeling of derived state. For instance, let's consider a basic FetchResource implementation. The naive implementation might look like:

class FetchResource extends Resource {
  @tracked isLoading = true;
  @tracked data = null;

  async setup() {
    let response = await fetch(this.args.url);
    let result = await response.json();

    this.isLoading = false;
    this.data = result;
  }

  async update() {
    if (this.isLoading) return;

    this.isLoading = true;

    let response = await fetch(this.args.url);
    let result = await response.json();

    this.isLoading = false;
    this.data = result;
  }
}

If you look closely though, you'll spot the bug: we're reading this.isLoading, then immediately setting it. This is invalid, because you're updating state that has been read.

The worry is that this will be a very common issue, precisely because we have this update hook and this notion of adding an imperative step in what should otherwise be mostly declarative. The solution is that rather than update the existing instance of the class, you need to create a new instance, which will have isLoading = false by default. Essentially this new model.

This is where I got the idea of thinking about Resources as "reactive classes". The mental model is this:

1. Resources are just classes, like any other class. They have a constructor, and they can also have a destructor tied to their parent.
2. They also track their constructor, and when something changes in the construction of the class, it creates a new instance.

This is a simpler mental model than "resources are a special object that gets an imperative lifecycle", is a bit more inline with the philosophy of autotracking, and guides people toward the correct solution more often, is the thinking.

That said, if there are common use cases where update and/or a "final destructor" like the one you're describing are needed, then maybe it's not the right direction.

On the related topic of teardown: In our app, we've long wanted to know when we could tell the server to cancel the real time subscription for a model, but we never had a way of tracking when all the components were done with the model. We've used PromiseProxy's which didn't let us know when they were destroyed (or at least we never figured it out). We're excited to use the teardown hook to implement tracking where we tell the store we're using the model on setup and when we're done on teardown. The store would increment/decrement a counter for the model and release the subscription when no resources were using it. Since it looks like the teardown hook is going away, could we achieve something similar in the constructor of the next instance? If so, how would we handle teardown when the resource is finally destroyed as a new constructor isn't called?

So, you would still be able to register destructors in general, so you could just do:

class MyResource extends Resource {
  @service store;

  constructor(owner, args) {
    super(owner, args);

    this.store.subscribers++;

    registerDestructor(this, () => {
      this.store.subscribers--;
    })
  } 
}

Yes, this would mean that mid-update there would be a point where you would possibly subtract and then add to the subscribers. I think it wouldn't actually, because destructors run lazily, so it would probably add first and then subtract later, but you get the point.

I think in most cases, you can probably model the behavior needed like so. If the operation on destruction really must happen only once though then it looks a bit trickier:

class MyResource extends Resource {
  expensiveCleanup = () => {};

  constructor(owner, args, prev) {
    super(owner, args);

    registerDestructor(this, this.expensiveCleanup);

    if (prev) {
      unregisterDestructor(prev, prev.expensiveCleanup);
    }
  } 
}

Not impossible, but definitely not intuitive. I think the question is, how common is this edge case? And in general, the cases like yours, where update seems to not cause issues and actually works as a way of modeling state?

[scottmessinger]

First, thanks as always @pzuraq for the thoughtful comment (and thanks @NullVoxPopuli for the thoughtful comment you posted earlier!).

The worry is that this will be a very common issue, precisely because we have this update hook and this notion of adding an imperative step in what should otherwise be mostly declarative. The solution is that rather than update the existing instance of the class, you need to create a new instance, which will have isLoading = false by default. Essentially this new model.

This is a great point. I hadn't run into that yet, but I'm sure it's only a matter of time before I would. Looking at my code, I think one reason I've avoided this is I read this.args.variableName and then set this.variableName and read this.variableName later in the function, so my reads always follow my writes. Reading your writes are okay, right?

However, this has got me worried that I've avoided this footgun by accident and it's a matter of time before I hit this problem. I see how this is not something we want people to experience!!

Yes, this would mean that mid-update there would be a point where you would possibly subtract and then add to the subscribers. I think it wouldn't actually, because destructors run lazily, so it would probably add first and then subtract later, but you get the point.

I haven't fully considered the edge cases, but I think you're right -- this would solve the problem. And, for cases where it has to run exactly once, I like that there's an escape hatch, though I agree it's definitely not intuitive.

In contemplating this new API, I tried to migrate the example you gave to version 2. Do I have this right?

class FetchResourceV2 extends Resource {

  // We don't know if we're loading on init as the args might have changed in a way
  // that wouldn't necessitate a re-fetch.
  @tracked isLoading = false
  @tracked data = null

  constructor(owner, args, prev) {
    super(owner, args)

    if (prev.data){
      // set the data to be the old data so we don't have a flash 
      // of nothing (if we end up refetching) 
      this.data = prev.data
    }

    if (prev.isLoading) {
      // Some sort of cancelation logic.
      prev.cancelRequest()
    }

    // If the arg relevant to the fetch has changed
    // We might pass a few args but only one necessitates a re-fetch
    if (prev.args.url !== this.args.url) {
      this._loadData()
    }
  }

  async _loadData() {
    this.isLoading = true

    let response = await fetch(this.args.url)
    let result = await response.json()

    this.isLoading = false
    this.data = result
  }
}

Even though we're setting this.data with prev.data (the same object), would we have a flash where we see Glimmer re-render (assuming data is rendered in the UI)?

[scottmessinger]

This is a great point. I hadn't run into that yet, but I'm sure it's only a matter of time before I would.

Today, I ran into the problem of updating state after reading it!

In thinking about the new API, if we're passing in prev, I think that would solve my use cases (assuming my use of v2 is correct above). I still just want to make sure we're not going to cause any re-renders by switching out the resource object (even if set this.data to prev.data and it's the same object in memory).

[pzuraq]

I still just want to make sure we're not going to cause any re-renders by switching out the resource object

I think that's an interesting new constraint. I guess the idea is that update() can decide whether or not to update some data, but if we always throw away the object or not, we don't get a choice?

What are the use cases for update that does not always update? Just to get some perspective on that

[scottmessinger]

What are the use cases for update that does not always update?

Great question! I think this falls into a few categories.

1. Passing metadata which could change and not necessitate a refetch. I don't have a great example of this atm, but I know I've run into this problem before (with non-resources). Let's say I do something like this:

@use model = new FetchModel(() => ["lesson", this.args.lessonId, this.args.currentTab])

And the resource does something like:

post(`/api/lessons/${lessonId}?currentTab=${currentTab}`

Again, this is a poor example, but the basic idea is we're fetching a resource but passing in some metadata due to some sort of business requirements. In this example, probably for analytics reasons.

2. A statechart: In the update method, a new arg might advance the state chart or it might not. The new arg might cause the state chart to remain in the same state.

3. Passing in a whole object but only reacting to some properties. IMO, the most common one people will run into: passing in a whole object when only one part should trigger the update.

// We pass in the whole lesson object
@use course = new FetchRelatedModel(() => [this.args.lesson, "course"])

// In the resource, we do something like this: 
lastPath = null
lastId = null

@tracked value = null

setup(){
   this._fetch()
}
update(){
   this._fetch()
}
_fetch(){
  let model = this.args.positional[0]
  let relatedModelName = this.args.positional[1]
  let path, key

  // This is grossly simplified, but something like this: 
  if (relatedModel == "course") {
    path = "api/courses"
    key = "courseId"
  }

  let id = model[key]

  if (this.lastPath !== path || this.lastId !== id) {
    this.lastPath = path
    this.lastId = id
    post(`${path}/${id}`).then(res => this.value = res)
  }
}

In this example, we're passing in the whole object (the lesson object) but only looking at a few properties in order to do the fetch. Given my understanding of auto tracking, the update function would rerun whenever anything in this.args.lesson updates. Thus, we want to store the last path and id and check if we need to refetch or if something unrelated to the path and id changed in the model.

This could probably be worked around but being more careful about what properties we're passing into the resource. However, I think this pattern would be common and we'd have to work hard to help people avoid it.

4. Updates where we append instead of replace the value For instance, in our query resource, we pass in the name of the query, the params for the query and the offset. When the offset changes, we fetch more records at that offset and append them to the results. Our current update method does something like this:

value = new TrackedArray([])

setup(){
  this.fetchQuery()
}
update(){
   if (this._isNewQuery()){
     this._fetchQuery()
   } else {
     this._fetchExistingQueryAtNewOffset()
   }
}

_isNewQuery(){
   // checks if our  query id and params have changed or if just the offset has changed
}

_fetchQuery(){
   fetch(...).then(res => this.value = res)
}

_fetchExistingQueryAtNewOffset(){
  fetch(...).then(res => res.forEach(el => this.value.push(el))
}

Let me know what you think of those examples!

[chrismllr]

Was referred here by @scottmessinger 👋 ,

Still digesting this chain, but off the bat, I can provide an anecdote based on recently picking up this library and building out our first Resources...

One of my use cases was wrapping the Apollo-client's watchQuery, where the update api came quite in handy for me: The watchQuery returns an Observable, which when our variables updated (new Resource arguments) we'd .refetch() the query instead of completely tearing it down.

The update() API enabled the following:

• A refetching state: design had a specifically different state when data is being refetched (leave old data visible while fetching new)
• Less subscribe/unsubscribe on the observable exposed by the apollo client

This is pretty specific to my Resource exposing an Observable; if the underlying Resource uses a stateless fetch, call, a fresh start with some results carried over could do well. But having update helped in our use case 👍

[josemarluedke]

@chrismllr You might want to check out the project I'm working on for apollo using resources: https://github.com/josemarluedke/glimmer-apollo

[chrismllr]

@chrismllr You might want to check out the project I'm working on for apollo using resources: https://github.com/josemarluedke/glimmer-apollo

Ha, figured someone must have done it!

Looks like the use of update/refetch is similar to what I had come to. I suppose passing the previous observable along to the next setup would do, but might lead to passing things along in the value which weren't meant to be public-facing per-se

[pzuraq]

@scottmessinger after some more thought, I think that actually the preventing-rerenders ability does not exist today, and cannot exist in general. This comes down to the basic mechanism of resources - they are pull based.

Let's say you have a resource that you access like so:

{{this.myResource.data}}

When you access the instance of the resource to read the data property, you are entangling the state of that resource. The next time the resource updates, it does not do so until it is accessed. The template already believes that the resource is dirty, and so it's already going to rerender.

The way to optimize updates is to forward the state from the previous instance to the next instance if nothing has changed. Then, when the rerender reads that state, it'll notice that it has not changed and not continue. So I think overall this design still works and covers all of the essential use cases in as efficient a way as possible.

[scottmessinger]

@pzuraq If I'm understanding this right:

Then, when the rerender reads that state, it'll notice that it has not changed and not continue

We wouldn't see any screen flashing or any DOM mutations. The VM would do a dirty check, see the value hasn't changed, and, thus not update the DOM. While some work would be done, no DOM work would be done. Because the state was the same between pulls, we wouldn't see a performance impact from this new approach. Am I understanding this correctly?

[pzuraq]

Correct, and that's the way it's working currently with the update hook. When the update hook runs, the resource currently returns the same instance, but from the VM's perspective it's a new object/value. So actually returning a new object will be the same from the VM's perspective.

[NullVoxPopuli]

I've added this lifecycle-less resource to https://github.com/NullVoxPopuli/ember-resources#resource (v3.1.0) if anyone wants to play around with how the implementation behaves in their specific scenario. Would love to find and discuss more specific examples of where the lack of lifecycles gets weird. I think for most things, it should be fine though. In my tests, it actually ended up being fewer lines of code, which is nice. :D