Make Service objects plain ES classes

[mehulkar]

The main purpose of services is to be an injectable singleton. It does not seem to be gaining much by extending from FrameworkObject which inherits from CoreObject. The main benefit of this hierarchy is the Observable mixin, which provides things like this.get and friends. I think somewhere, Service also mixes in Evented, but I couldn't find it.

[mehulkar]

And if they're going to be plain classes, I wonder if we need anything other than some utils to register and inject these classes correctly. E.g. app/services/foo.js could may just be able to export class Foo {}.

[mehulkar]

Service-specific core code is pretty thin already, but if Ember can go in this direction, I'd want to make it an optional feature, similar to #587.

[buschtoens]

I am very much in favor of this! It'd be a great step into the direction of being able to fully pull out business logic into pure vanilla TypeScript / JavaScript packages.

I think somewhere, Service also mixes in Evented, but I couldn't find it.

AFAIK it does not. However, many core service implementations, like the RouterService do.

I think the ability to apply mixins in the first place, is also only possible with CoreObject, but I might be mistaken. I always try to avoid mixins anyway; for instance, instead of mixing in Evented, you can use the @ember/object/events utility function on the class instance.

And if they're going to be plain classes, I wonder if we need anything other than some utils to register and inject these classes correctly. E.g. app/services/foo.js could may just be able to export class Foo {}.

The only thing that's needed in a class to be instantiable by the resolver / container / DI system is a static create method.

Then there's also the "destroy" lifecycle hooks interface that needs (?) to be implemented, but #580 would nicely replace that with a utility functions based approach (like @ember/object/events) as well.

[buschtoens]

I did a quick Ember Twiddle to implement a plain Service:

import { inject as service } from '@ember/service';

export default class FooService {
  @service bar;

  constructor(injections) {
    Object.assign(this, injections);
  }

  static create(injections) {
    return new this(injections);
  }
}

injections is an object, which has the owner injection and any further injections defined via Application#inject.

[Gaurav0]

Just thinking, we probably don't want to have to explain to users the mechanics of how to make their service work with Ember.

Here are some approaches we can take:

1. Do some magic. We detect if the Service does not extend from @ember/service and patch the class's constructor and add a static create method.

2. Make it slightly more explicit with a decorator that lets the user know it will be patched, but do the same thing.

3. Have the decorator actually do the patching. I'm concerned this may not be possible with legacy stage 1 decorators or the current proposal, although old stage 2 decorators, still supported by Babel, could have done this.

4. Extend from a new superclass. Perhaps @glimmer/service..

5. Bring back mixins with something like mixwith

There may be other approaches I haven't thought of.

[chriskrycho]

The lightest-weight move is to have a trivial base class and have the constructor receive owner. That's what we do with @glimmer/component: it receives owner and args. Doing the same here would allow for a very lightweight system, and one that could be straightforwardly migrated to for anyone who has already followed the classic class decorator workflow.

(Adding magic into the system is effectively going backwards here. What we want is something that is pleasant to use but also sufficiently explicit that it's easy for users to understand when they want to, and which retains the benefits of static analyzability.)

We could basically implement the lightweight base class approach by making something very close to @buschtoens's be the base class (it could probably be tweaked slightly to match the owner-driven API that Glimmer components have), and just have folks sub-class from it with that minimal API surface.

One of the upsides to the approach @buschtoens outlined, too, is that users who want to build their own base class or just implement it directly per-service can: as long as it matches a well-defined interface, you're off to the races.

• eliminating mixins and CoreObject is 👍
• requiring further decorators when an extremely simple base class will do the trick is 👎
• putting in some other form of mixins is also 👎
• doing magical transforms based on the file location is extremely 👎

[Gaurav0]

@chriskrycho I was enumerating options, not weighing in. I think your idea is a good one. The slight downside is that there will be duplicated logic between @glimmer/component and our new superclass (@glimmer/service ?).

[chriskrycho]

Yep, I was just enumerating the actual trade offs around the options you enumerated!

As regards a base class, I don’t think that level of duplicated functionality is something to worry about. The specifics are different (including arguments vs. not) and an overemphasis on eliminating every point of repetition of any sort is its own problem. The key for this kind of design is to provide good primitives (a simple and clear way to hook into the DI system), and then a nice abstraction on top of that (e.g. thin base classes which get rid of boilerplate), while maximizing flexibility for each type as appropriate. Only share what is actually shared—which in this case is the primitives, not the (base) classes built with those primitives.

I’m mulling on some delegate-based and manual-registration based approaches here as well, will share if I land on anything I think is actually good.

[gossi]

I'm super much in favor of going into this direction!

Apparently, I think @mehulkar is right in saying it has some connection to the Evented mixin.

AFAIK it does not. However, many core service implementations, like the RouterService do

I think the ability to apply mixins in the first place, is also only possible with CoreObject, but I might be mistaken. I always try to avoid mixins anyway; for instance, instead of mixing in Evented, you can use the @ember/object/events utility function on the class instance.

@buschtoens I think I have to disappoint you. The inheritance chain is: Service < FrameworkObject < CoreObject. I couldn't find the actual mixin part of evented, but this line this.trigger(...) is telling that Evented is in use (afair Evented was/is (?) in use on most of @ember/* objects by default(?)).

I think many addons and still many code relies on Evented being there and a removal would break all that code. However at the same time I'm also vote for this to be removed from ember, there are enough libraries out there, that can be used instead ;)

On the other hand if the new base class is @glimmer/service instead of @ember/service then this problem is evaded. I actually like this, so you don't carry over all that bloat and keep it lightweight.

[hjdivad]

The lightest-weight move is to have a trivial base class and have the constructor receive owner.

@chriskrycho lighter still is to have a constructor interface and not even consume the user's one base class for services.

[mehulkar]

@gossi Evented gets mixed into RouterService further down the page, so I think @buschtoens may be right that it’s not a default. https://github.com/emberjs/ember.js/blob/23d3ff0436e03dff172efa3360ad09cbec98f80f/packages/%40ember/-internals/routing/lib/services/router.ts#L410

@chriskrycho im not convinced a base class is needed. The thing that provides the owner to the constructor is whatever resolved the class and understands that it’s a Service class. Which is really a proxy for saying “this should be injectable and this should be a singleton”. I don’t know what the right abstraction is here, but I keep coming back to the idea of explicit registrations and injections. That wouldn’t quite work out of box for lazy instantiations, as services are, but an app boot hook that includes some lines of code that do these registrations could easily adapted to do the same thing for ES classes that don’t extend from anything.

[Panman82]

FWIW, I prefer base classes. That way I can open a file and see what it is supposed to be, rather than relying on file location to determine type. Additionally I would think a base class would be better for third party tools inspecting files to then know what they're exporting.

[pzuraq]

I think there are two things we can do to approach this problem in parallel:

1. Add a DI manager that allows users to specify how a value is instantiated when being created via DI. This would replace the static create method interface in the long run, and would also allow non-Ember constructs/"services" (e.g. Apollo, Redux store, etc) to be more easily instantiated and registered.

2. Add an optional feature to make Service a base class that does not extend from EmberObject. This will allow folks to move away from EmberObject specific APIs, and help to de-tangle EmberObject from the ecosystem over time.

Doing 1 will set us up for the long term, and 2 will help us migrate in the short term. The Service class can also be re-rationalized in terms of 1 (it's a simple JS class that has a DI manager associated). This doesn't commit us to removing it in the short term, but makes it would make it much easier to do so if we want to in the future.

[Gaurav0]

Add an optional feature to make Service a base class that does not extend from EmberObject. This will allow folks to move away from EmberObject specific APIs, and help to de-tangle EmberObject from the ecosystem over time.

@pzuraq I would rather have a different base class. Optional features are already making things more difficult for addon authors that can't know if they are turned on or off (there is no public api, and even if one is added they have to test for both scenarios).