Explicit Service Injection

[NullVoxPopuli]

rendered

[webark]

@NullVoxPopuli Where does the class name come from? Is it just the path camel cased with the type tacked on? Would it be something you import? Is it the class name you define in the individual file that would get pulled out?

[flashios09]

hi @NullVoxPopuli ,
i like the idea 👏
this will add some imports but it will make the code more clear/readable
i wish if we can do the same with a model
so instead of this:

this.store.findAll('post', '...')

we have:

@service(StoreService) store;
// i prefer the pluralized form, `posts` not `post` for model name
@model(PostsModel) posts;
// ...

doSomething() {
    this.store.findAll(this.posts, '...');
    // or
    this.posts.somePostsModelMethod('...');
   // maybe
   this.posts.findAll('...');
   // a class cased form for model name `Posts` or `PostsModel`
   this.PostsModel.findAll('...');
}

[buschtoens]

A few tangents regarding TypeScript.

Currently I (we?) do this for TypeScript:

import Service, { inject as service } from '@ember/service';
import BarService from './bar';

class FooService extends Service {
  @service bar!: BarService;
}

Alternatively you can get around the extra import of BarService, if you provide the service name explicitly and use the old Ember Object Model:

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

class FooService extends Service.extends({
  bar: service('bar')
}) {
}

This works because of the clever registry pattern. The reason you have to use the Ember Object Model is that decorators still can't yet change the type signature: microsoft/TypeScript#4881

Once they could, the following should type-check and infer the service class automatically:

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

class FooService extends Service {
  @service bar;
}

For the time being, if you don't want to or can't use the Ember Object Model, but dislike explicitly importing the injtectee class, you could use the trick that we discussed in machty/ember-concurrency-decorators#50: Use a Babel transform to convert class property assignments to decorated properties.

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

class FooService extends Service {
  bar = service('bar');
}

// gets transformed into

class FooService extends Service {
  @service('bar') bar;
}

I have a transform for it ready, but I need to battle-test and optimize it further:
babel-plugin-transform-class-property-assignment-to-decorator

[gossi]

@buschtoens

Alternatively you can get around the extra import of BarService

if I understand this RFC correctly, it is about explicitely importing the respective class and exchange the string lookup for the class definition.

Second is: As much as we love typescript, this RFC must work for JS and as such the registry pattern from e-c-ts isn't available in pure js land. So with this RFC TS code will look like this:

import Service form '@ember/serivce';
import NotificationsService from 'my-project/services/notifications';

class Foo extends Service {
  @service(NotificationsService) notifications: NotificationsService;
}

at least until decorators can mutate the type definition for properties (then we can get rid of the doubled written class name).

[buschtoens]

@gossi This is exactly why I raised these points. While Ember is committed to not pushing TypeScript onto anybody and offering first-class support for JS, we are equally committed to offering first-class support for TypeScript as well.

I find it important that, as long as decorators cannot change types in TS or using a Babel transform is not a community-accepted and agreed upon opinion, we do not deprecate string lookups, as this could worsen ergonomics for some TypeScript users.

---

Edit: I actually believe we should never deprecate string lookups, as this would break the automatic inferral, when decorators can change signatures, i.e.:

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

class FooService extends Service {
  @service bar;
}

[buschtoens]

One further observation: The Ember Resolver / Container system is a bit of magic in the background. You don't always know where the backing injectee class is actually located or it might not be accessible to the code you are authoring.

There's also nothing preventing you from registering the same class with multiple names or even generating the names at runtime.

Admittedly these are uncommon edge cases.

[lougreenwood]

Also, by requiring explicit class name use, we're coupling the class to a specific service, but this shouldn't be a concern of the class:

Dependency injection is one form of the broader technique of inversion of control. The client delegates the responsibility of providing its dependencies to external code (the injector). The client is not allowed to call the injector code;[2] it is the injecting code that constructs the services and calls the client to inject them. This means the client code does not need to know about the injecting code, how to construct the services or even which actual services it is using; the client only needs to know about the intrinsic interfaces of the services because these define how the client may use the services. This separates the responsibilities of use and construction.
https://en.wikipedia.org/wiki/Dependency_injection

I mean, if we're going to import the ClassName, why not instantiate the service singleton and skip the service() container lookup all together.... But we all know that's a bad idea as it removes IoC.

So whilst I like the look of this as purely a nicer way to write TS classes in Ember right now - it's not really DI any more since the explicit class name is required (no more IoC).

So it seems that if the goal is to improve typing in TS, proper TS support for decorators will give us that.

For newbies that get confused by strings, maybe we need to better teach the newbies about DI and why a string name is about as explicit & accurate a DI lookup name should be to maintain proper separation.

[buschtoens]

@lougreenwood I totally agree with the point you're making regarding isolation of concerns.

However, I think @NullVoxPopuli was not aiming at better TypeScript support, but instead better "cmd+clickability" support for generic JS IDEs / users that don't use TypeScript.

^{Please correct me, if I am wrong.}

[lougreenwood]

yeah - you're right, I was getting ahead of myself - sorry :D

But IMO, we also shouldn't start hacking apart established patterns for nicer IDE support - our solution should "work with the patterns" as well as "using the platform".

So if I understand JS & IDEs correctly... Since JS is not typed, and by it's nature DI de-couples - then there's no "platform" mechanism which is true to DI other than typing (and decorators changing function signature) which allows nice IDE integration?

[NullVoxPopuli]

@webark the class would be an import of the service itself :)

import Component from '@glimmer/components';
import { inject as service } from '@ember/services';
import MyService from 'appmame/services/my-service';

export default class extends Component {
  @service(MyService) myService;
}

:)

[NullVoxPopuli]

@buschtoens the registry pattern could be used with classes as keys, couldn't it?

[NullVoxPopuli]

then there's no "platform" mechanism which is true to DI

If only we had interfaces we could use instead :)

[NullVoxPopuli]

Renamed RFC due to technicality in DI definition

[buschtoens]

the registry pattern could be used with classes as keys, couldn't it?

@NullVoxPopuli Yes, of course it could. But that's not the point I was trying to make. 🙂

Switching to a class-based registry / decorator and deprecating the string-based one, means that the future TypeScript ergonomics will be much worse, because you can't any more use the property name to infer the injection, as show in #502 (comment).

[NullVoxPopuli]

Gotchya, so string lookup stays :)

[webark]

the class would be an import of the service itself

Thanks for clearing that up!

So there’s a pattern where you extend a dependencies service, to either overwrite or extend the dependent service.

When this is done, all of your existing references are updated.

With direct imports, would you always import a dependents service from the addons merged “app” space, even if it doesn’t exist? Or would you need to go and update all of your imports where you had imported them from the depended addon?

[NullVoxPopuli]

would you always import a dependents service from the addons merged “app” space, even if it doesn’t exist? Or would you need to go and update all of your imports where you had imported them from the depended addon?

it should resolve to the same thing in both scenarios, afaik

[buschtoens]

With direct imports, would you always import a dependents service from the addons merged “app” space, even if it doesn’t exist? Or would you need to go and update all of your imports where you had imported them from the depended addon?

it should resolve to the same thing in both scenarios, afaik

An addon that has a service re-export in its app tree would make the service available in the host app, via the resolver, however the file would not be physically "there", meaning that "cmd+click" would fail, while the import would work at runtime.

---

I don't want to explode the scope here, but this segways into another interesting problem: What is the "official" way (pre and post this RFC) to override / clobber an addon's service (from another addon possibly)?

One way is using ember-addon.after in the package.json of the overriding addon to ensure that its app tree clobbers the app tree of the overridden addon. You could do the same from the host app as well.

One major hazard of this approach is emberjs/ember-cli-babel#240: If you use a file extension other than js, the clobbering might fail non-deterministically.

[NullVoxPopuli]

I don't want to explode the scope here, but this segways into another interesting problem: What is the "official" way (pre and post this RFC) to override / clobber an addon's service (from another addon possibly)?

maybe this RFC should be held off until embroider ships ;)
(though, I need to see if embroider would actually fix that problem)

but, I don't think we should continue to have clobbering of services. It's hard to debug.
if I import a service form an addon, I want that to be the actual location so I can ctrl/cmd click it and see what it is.

[webark]

An addon that has a service re-export in its app tree would make the service available in the host app, via the resolver, however the file would not be physically "there", meaning that "cmd+click" would fail, while the import would work at runtime.

this just goes against the main motivation of “enable "go to definition" support from service definitions so developers can more easily discover the where and how their service is defined.”

For using services in addons, it seems just as magical also, and more confusing around “is this a singleton” especially if you could import the same service from the invisible “app” space, and where it lives in the addon’s “addon” space in the same app.

[mehulkar]

This RFC should also probably provide some text about how to register/inject services in component rendering tests.

[wagenet]

Where does this stand?

[NullVoxPopuli]

@chancancode has a user-land addon that explores this space. 🤔

and @wycats has some thoughts, too.

There is also: emberjs/ember.js#20095

• where I said I'd try to prototype this out soon (though, in that comment, I said not ™️, but it's def ™️ at this point) -- I got distracted

[wagenet]

This is being moved to the Exploring state.

[lifeart]

While reading https://netbasal.com/lazy-load-services-in-angular-bcf8eae406c8

I think we could have something like this https://gist.github.com/lifeart/fbcc7bd8747562aa85d79b42ca991493

In short, all properties on lazy service may be promisified by default, with ability to call toSync to get "reference" service object.

thanks to @NullVoxPopuli navigating me here.

Some version of provided gist:

class Bar {
    doSomething() {
        console.log('do something');
    }
    name: string;
}

type PromisifyProps<T> = {
    [P in keyof T]: T[P] extends (...args: infer A) => infer R ? (...args: A) => Promise<R> : Promise<T[P]>;
};

// a function to accept service load using import
// and it should return same type as service but all it's methods and properties should be promises
// we can use this function to make lazy loading of services
// under the hood we use proxy to make all methods and properties to be promises
function lazyService<T extends object>(service: () => Promise<T>): PromisifyProps<T> & {
    toSync(): Promise<T>;
} {
  let loadedService: T;
  const proxy = new Proxy({}, {
    get(_, prop) {
      return new Promise(async (resolve, reject) => {
        if (!loadedService) {
          try {
            loadedService = await service();
          } catch(e) {
            reject(e);
          }
        }
        if (prop === 'toSync') {
            return resolve(loadedService);
        }
        const value = Reflect.get(loadedService, prop);
        if (typeof value === 'function') {
          return resolve((...args: any[]) => Promise.resolve(value.apply(loadedService, args)));
        } else resolve(value);
      });
    },
  });

  return proxy as PromisifyProps<T> & {
    toSync(): Promise<T>;
  }
}

class Foo {
    bar = lazyService<Bar>(() => import('./bar'));
    async onClick() {
        // auto-load and invoke service method
        await this.bar.doSomething();
        // get service property
        const name = await this.bar.name;

        // convert async service to sync (auto-load)
        const sync = await this.bar.toSync();
        const secondName  = sync.name;
        
    }
}

[ef4]

While there's nothing that would preclude that kind of auto-lazy service for someone who wanted it, I don't think it makes much sense as a default or encouraged pattern. There are almost always better places to do the lazy loading.

For example: if the service will only gets used based on certain URLs, it would get taken care of automatically by route-based splitting, with no need to juggle promises in the component.

For another example: if a service is only needed when the user clicks on a particular button, it's actually not great to do the lazy loading after they click. That puts the loading into their perceptible critical path. It's better if the service is already loading the background before they click. It costs almost nothing as long as you do it after rendering the critical path. For that you'd probably want the service itself to have a small shim that's part of the initial payload that is responsible for lazy loading the rest, preferable using idle timing (which would be nice to do given something like #957).

[ef4]

This was discussed at the spec meeting this week, the main open point of discussion was wether we want to keep the decorator syntax despite typescript still not inferring it, or use a field assignment syntax instead that would infer correctly (with an explicit this):

  cookie = service(this, CookieService);

[chancancode]

Expanding on @ef4's comment above:

Today, service lookups are keyed on the tuple (owner, "string-key"). This means that:

1. we can't easily tell from the import graph which services are needed by what modules, thus we have to eagerly load/register all services upfront
2. in order to provide services that are scoped to engines (and that is the only kind of scoping we provide today), we have to create a "child owner" for each engine (the first part of that key tuple)

In Polaris, we plan to solve both of these problems by changing the lookup key to (owner, SomeJavaScriptValue).

1. in order to inject the service, you would have to acquire it somehow, typically by importing the module where the service is defined
2. standard JavaScript features allows you to control the scoping of the service, for example:
   1. un-exported values can only be used locally with the module
   2. exports map in package.json allows you to keep the value private within the package
   3. packages can ensure they share (or not) services with the appropriate peerDependencies relationship
   4. you can always register that as a global, pass it as argument, {{yield}} it to child components, etc

In terms of practical day-to-day usages, it will probably look something like this:

// app/services/session.js
import { setOwner } from "@ember/owner";
import { tracked } from "@glimmer/tracking";

export default class SessionService {
  // Alternatively, we can still provide a superclass in the framework
  // to deal with this boilerplate, just have to be a different import
  // and not subclassing from `Ember.Object`
  constructor(owner) {
    setOwner(this, owner);
  }

  @tracked currentUser = null;
}

// app/components/menu.js
import { service } from "@ember/service";
import Component from "@glimmer/component";
import Session from "my-app/services/session";

export default class Menu extends Component {
  @service(Session) accessor session;
}

...or...

// app/components/menu.js
import { service } from "@ember/service";
import Component from "@glimmer/component";
import Session from "my-app/services/session";

export default class Menu extends Component {
  // About the same number of characters but possibly work better with TypeScript today
  session = service(Session, this);
}

For the primitives, we will need:

// The definition of "Owner" in this context is different/more relaxed than the traditional one
// We only need it to be a WeakMap-key-able, that should probably be rectified across the framework
export type Owner = object;

export type ServiceDefinition<T> = /* ...defined later... */;
export type ServiceInstanceType<S extends ServiceDefinition<any>> = /* ...defined later... */;

type InstantiatedServices = WeakMap<ServiceDefinition<any>, ServiceInstanceType<ServiceDefinition<any>>>;
const Services = new WeakMap<Owner, InstantiatedServices>;

export function lookupService<S extends ServiceDefinition<any>>(owner: Owner, definition: S): ServiceInstanceType<S> {
  let services = servicesFor(owner);
  let service: ServiceInstanceType<S> | undefined = services.get(definition);

  if (service === undefined) {
    service = instantiate(owner, definition);
    services.set(definition, service);
  }

  return service!;
}

type OverriddenServices = WeakMap<ServiceDefinition<any>, ServiceDefinition<any>>;
const Overrides = new WeakMap<Owner, OverriddenServices >;

export function overrideService<S1 extends ServiceDefinition<any>, S2 extends ServiceDefinition<any>>(owner: Owner, definition: S1, override: S2) {
  if (DEBUG && servicesFor(owner).has(definition)) {
      throw new Error(`Cannot override service ${inspect(definition)} after it has already been instantiated`);
    }
  }

  let services = servicesFor(owner);
  let service: ServiceInstanceType<S> | undefined = services.get(definition);

  if (service === undefined) {
    service = instantiate(owner, definition);
    services.set(definition, service);
  }

  return service!;
}

function servicesFor(owner: Owner): InstantiatedServices {
  let map = Services.get(owner);
  
  if (map === undefined) {
    map = new WeakMap();
    Services.set(owner, map);
  }

  return map;
}

function instantiate<S extends ServiceDefinition<any>>(owner: Owner, definition: S): ServiceInstanceType<S> {
  /* ...defined later... */
}

On top of which we can build the convenience API.

From the primitive's perspective, we can accept any WeakMap-key-able (object) as a "service definition" (or "service key"). However, we need to know how to instantiate that key into the service instance. Usually, we would have some kind of manager for this purpose. On the other hand, we don't really care what the instance type is – for example, boolean as a service (service(isMobile, this) service) should work just fine.

[ef4]

Linking to @chancancode's addon which explores this design and has some discussion issues around open questions: https://github.com/chancancode/ember-polaris-service

[ef4]

Important design issues uncovered by experimentation work:

• js module used as service token can lead to exception due to module cycles chancancode/ember-polaris-service#18
• pattern for using a service in an addon based on an interface chancancode/ember-polaris-service#19

[les2]

there may be cases where people do not want or care about static typing and would like to rely on "duck typing" -- any object that responds to the "quack()" message shall be a "duck".

should the validation be configurable globally or per registration? or is it per lookup?

e.g., in testing, i might inject a sinon.mock() instance that is programmed to respond to a subset of methods and i might not use the abstract base class at all.

[NullVoxPopuli]

duck typing specifically I think has to be a separate RFC, because achieving that in a language that doesn't support such a thing will take an RFC as long as this one.

I've called this "looking up by shape" elsewhere in this existing RFC

programmed to respond to a subset of methods and i might not use the abstract base class at all.

that is fine as long as the key is the same, and/or the key-hierarchy is the same. the keys are independent of the implementation (outside of initial instantiation)

[NullVoxPopuli]

Some implementation / example demos here: https://github.com/NullVoxPopuli/ember-rfc-502-playground/tree/main/app/demos

[BoussonKarel]

When I click on the Rendered link of this issue, it's throwing a 404:

[runspired]

Important design issues uncovered by experimentation work:

something I'd note - no referential token intended to be used by multiple packages is safe. Even same version, if a dep is ever duplicated you have a different token. There are only 2 exceptions to this: Symbol.for (not Symbol) and strings. Also note that Symbol.for is not safe from a typescript perspective - only strings satisfy the unique reference + unique type constraint if you care about the potential for code duplication.

We hit this a lot in WarpDrive, and ended up writing special utils to handle defining and retrieving references for objects, strings and symbols. (For objects, we keep a global registry with a string key, first reference registered wins).

[NullVoxPopuli]

for all situations where you would have duplicates, you fall in to two categories

• the project's deps are messed up (either peer, externals, or package manager issue)
• you do want separate copies of services, because perhaps you have multiple majors / versions of a dependency (and thus the service it provides), and the code for those multiple dependencies is only written for the specific version of the service that was published with its major. if we were to use strings, whichever copy of the dep is out of date would fail with runtime errors

so, I dare argue the opposite: referential keys gets you more stability, even if it means you don't always get the same copy of a service that every copy of a dependency has (the problem is the duplicate deps)

[runspired]

the project's deps are messed up (either peer, externals, or package manager issue)

this is aspirational but no package manager today is capable of creating accurate dependency trees (isolated or hoisted) without using duplicate copies of the same version of a package.

I'm not necessarily arguing for strings here - I'm just pointing out where things are likely to get a little rough. The ember ecosystem tends to do something I have not seen in any other ecosystem so far - have community packages that depend on other community packages as peers as part of one global ecosystem. That makes us far more susceptible to package reference issues imo than say tanstack or millionjs deps that tend to pull from their own ecosystem only.

[NullVoxPopuli]

thankfully, what I'm proposing here does not get rid of or discourage the use of string keys -- as that's the existing behavior.

If folks want to keep using the old way, that's fine.

This RFC is for net new, and since the string lookup implementation is like.. 2 lines of code, I see no reason it can't stick around

Folks today can already use private services, and services-by-reference if they wish (I do this with utilities that ship as optional imports in ember-primitives (but also the code to do it without a dep is very small))

I still maintain that string services should be the exception for cases where users can't get their deps correct, and I am proposing the default guides materials be updated to use non-string keys

[NullVoxPopuli]

TODO for me:

• PR to the guides for how to manage state for library authors, and app authors a like
  • these will not use services
  • touch an the different lifetimes
    • explain how to wire up an instance to the container, if needed
    • explain how to wire up destruction, if needed
    • wire up to the container -> singleton
    • wire up to the module -> singleton, but be careful (also might be what you want)
    • wire up to a component -> unit-testable state, not singleton
    • how to idempotent

[gossi]

Hmm, I had some concerns about the names. It took me a while, since @service didn't feel right anymore and @NullVoxPopuli demo shows, this is now about any reference.

API wise I think, this is a better fit:

import { lookup, register, inject } from '@ember/di';

potentially speaking (I know this goes to far) but to play the surface:

import { getContainer, type Container } from '@ember/di';
import { doBusinessLogic } from '#business-logic';

class MyComponent {

  doSomeBusinessLogic = (someObject: object) => {
    const container = getContainer(this);

    doBusinessLogic(someObject, container);
  }
}

at which point one can realize the DI system is no longer a necessity for ember to function, people can choose something externally (I ignored older setups, I know).

I wanna project the idea here, to recommend something in the wider ecosystem, than to maintain our own, which binds capacity. Or to seek agreement/common standard as they did for StandardSchema.

PS. I still know this from my php times with PSR-11: Container interface

[NullVoxPopuli]

I took a stab at some docs: ember-learn/guides-source#2237
(which, @gossi cover the scenario you described)