NullVoxPopuli / ember-resources

An implementation of Resources. Supports ember 3.28+
https://github.com/NullVoxPopuli/ember-resources/blob/main/docs/docs/README.md
MIT License
91 stars 37 forks source link

Intent to deprecate: The class-based Resource. #1056

Closed NullVoxPopuli closed 6 months ago

NullVoxPopuli commented 7 months ago

[!NOTE] this is the second topic of deprecation in planning for ember-resources@v7. The other set of things to look at is over here: https://github.com/NullVoxPopuli/ember-resources/issues/1046

Looking for feedback here before following through with this deprecation, as I know some big players are using class-based resources -- but in part because the existed before the current design of resources existed.

There a few related motivations:

Specifically, as a migration path, focusing on "The spirit of class-based resources can be implemented in userland" can be done in a couple of ways.

[!NOTE] These examples are highly dependent on your use case. If you have a use case that's not covered year, please comment!

having a value update when args change (part 1) ```ts class Doubler extends Resource<{ positional: [number] }> { @tracked num = 0; modify([passedNumber]: [number]) { this.num = passedNumber * 2; } } ``` > [!WARNING] > This test demonstrates (ab)using the `modify` hook as an _effect_, which resources are _against_ causing side-effects outside of their own context. Here the `modify` hook has setting tracked data that the class controls (technically in its own context), but this is conceptually similar to inlined function-resources setting data outside of their own context. **The way to do this yourself**: ```js import { helper } from '@ember/component/helper'; class Doubler { constructor(args) { this.args = args; } get num() { this.args[0] * 2; } } let CACHE = new WeakMap(); function ensureCached(positional) { let instance = CACHE.get(positional); if (instance) return instance; instance = new Doubler(positional); CACHE.set(positional, instance); return instance; } export const doubler = helper((positional) => ensureCached(positional)); ``` the `doubler` classic helper would be what is invoked now instead of the old `Doubler`. classic helpers are already usable in templates, and using them in JS can be done with the [`helper` util](https://ember-resources.pages.dev/funcs/util_helper.helper), which reduces the boilerplate over `invokeHelper`. ```js class Demo { @tracked something = 3; doubled = helper(this, doubler, () => [this.something]) get theValue() { return this.doubled.num; // 6 } } ``` This technique allows you to keep all your public API the same, but implementation can be way simpler if you're in a place to change all the public API / how the args are passed in. For example, if you only used your resource in JavaScript, you may not need a resource: If you either: - make a class for your args object - or make all your args arrows, you can do this: ```js import { link } from 'ember-resources/link'; class Doubler { constructor(inputFn) { this.inputFn = inputFn; } get num() { this.inputFn() * 2; } } class Demo { @tracked something = 3; @link doubler = new Doubler(() => this.something); get theValue() { return this.doubler.num; // 6 } } ``` Uses [link](https://ember-resources.pages.dev/funcs/link.link) (note that link is an abstraction over some framework wiring that can get verbose -- it's not "needed", but the alternative is verbose)

How do you use resource to wire up a native class?

The old behavior didn't receive the args in the constructor, as it followed the ember-modifier pattern of only having a modify hook.

To get the spirit of that behavior back, you may want something like this:

import { use, resource } from 'ember-resources';

class MyDoubler {
    @tracked num; 

    modify(input) {
      this.num = input;
    }

    // not required, if you don't want
    destroy() {}
}

function Doubler(inputFn) {
  let state = new MyDoubler();

  return resource(({ on, owner }) => {
    setOwner(state, owner);

    // not required if you don't want
    on.cleanup(() => state.destroy());

    // reminder than this return function
    // is a separate reactive context,
    // so that we don't 
    return () => {
      // NOTE: this is a side-effect, and side-effects
      //       often cause infinite revalidation situation and should be avoided.
      state.modify(inputFn());
      return state;
    };
  });
}

class Demo {
  @tracked something = 3;

  @use doubler = Doubler(() => this.something);

  get theValue() {
    return this.doubler.num; // 6
  }
}

[!CAUTION] this is a side-effect, and side-effects often cause infinite revalidation situation and should be avoided.

A hack around this is to use an async IIFE or async function, but it's unergonomic, and relies on timing happenstance with the reactivity system and rendering.

async modify(input) {
  await Promise.resolve();
  this.num = input;
}

(noting that nothing awaits modify) or

modify(input) {
  (async () => {
    await Promise.resolve();
    this.num = input;    
  })();
}

or

modify(input) {
  this.update(input);
}

async update(input) {
    await Promise.resolve();
    this.num = input;    
}

Native classes and avoiding modify

import { use, resource } from 'ember-resources';

class MyDoubler {
    constructor(inputFn) { this.inputFn = inputFn; }

    get num() {
      return this.inputFn() * 2;
    }

    // not required, if you don't want
    destroy() {}
}

function Doubler(inputFn) {
  let state = new MyDoubler(inputFn);

  return resource(({ on, owner }) => {
    setOwner(state, owner);

    // not required if you don't want
    on.cleanup(() => state.destroy());

    return state;
  });
}

class Demo {
  @tracked something = 3;

  @use doubler = Doubler(() => this.something);

  get theValue() {
    return this.doubler.num; // 6
  }
}
gilest commented 7 months ago

An extracted package that supports the class-based Resource API would be useful as a migration step

Could be similar to @ember/render-modifiers with a clear warning not to use it for new code

🤔 I guess if that boilerplate is small enough it might be easier just to keep locally

SergeAstapov commented 7 months ago

@NullVoxPopuli pardon my ignorance, what is the way to lookup a service in a non class-based resource? e.g. we have some resources that abstract complex data fetching so we write reactive code in a template and make resource lifecycle bound host component.

how could you e.g. inject store service from Ember Data in a non class based resource?

gilest commented 7 months ago

how could you e.g. inject store service from Ember Data in a non class based resource?

Function-based resources have access to the owner of the object they're mounted on.

@use myData = resource(({ on, owner }) => {
  const store = owner.lookup('service:store');

  on.cleanup = () => {};
});

Inline docs ref

mabab commented 7 months ago

In this case, it would be nice to have a codemode to switch to Native classes avoiding modify. I'm a little bit addicted to class resources as an encapsulation of some context.

johanrd commented 7 months ago

@NullVoxPopuli Hi, thanks for the post. I think this can be a very good move.

TBH, I have been struggling quite a lot in understanding the concepts of resources. It has felt like the transition to Octane provided an elegant simplification in DX and mental model, whereas my attempts of using resources hasn't really gotten into my fingers yet. In retrospect, perhaps some of my confusion has been due to the unclear concerns ("teachability") regarding mix-up of function-based and Class based resources.

les2 commented 7 months ago

Personally, I like the readability of injecting services using decorators. Using owner.lookup is a code smell imo.

NullVoxPopuli commented 7 months ago

I think I want to extend the link API so that using plain classes is a bit nice, for example:

import { resource } from 'ember-resources';
import { link } from 'ember-resources/link';

class DisplayRepo {
  constructor(private _socket: () => WebSocket) {};

  @cached
  get socket() {
    return this._socket();
  }

  @service declare store: StoreService;
  @service declare intl: IntlService;

  doTheCleanup = () => { ... };

  // etc

}

function GetData(socket: () => WebSocket) {
  return resource((r) => {
    let instance = new DisplayRepo(socket);

    link(instance, r);
    r.on.cleanup(() => instance.doTheCleanup());

    return instance;
  });
}

What are folks thoughts on this? (or something similar)

The main thing is that you don't lose the ability to use plain classes in ember (that's worked for ages!), but there should probably be an ergonomic way to wire them up.

SergeAstapov commented 7 months ago

@NullVoxPopuli as above is just set of functions, how link would know about owner? And how would it know about different instances of engines and app(s)?

NullVoxPopuli commented 7 months ago

how link would know about owner?

Same way it does today, via getOwner: https://github.com/NullVoxPopuli/ember-resources/blob/main/ember-resources/src/link.ts#L124

And how would it know about different instances of engines and app(s)?

It doesn't need to, because it comes from the call-site waaaaaay down in helper-manager land. We have the owner here, in the manager for function-based resources, and we'd do setOwner on the object passed to the resource's callback (r in the above example)

NullVoxPopuli commented 7 months ago

An extracted package that supports the class-based Resource API would be useful as a migration step

@gilest, yes, this is a great idea -- it's all still built on public API, and can be supported on that public API from 3.28+

it would be nice to have a codemode to switch to Native classes avoiding modify

@mabab I (or someone!) could give that a go

NullVoxPopuli commented 7 months ago

I've added this issue to the v7 plan quest issue: https://github.com/NullVoxPopuli/ember-resources/issues/1061

lemme know what you think!

NullVoxPopuli commented 6 months ago

here it is: https://github.com/NullVoxPopuli/ember-modify-based-class-resource