New (TS-friendly) design for DI registration and lookup

[chriskrycho]

Multiple times recently, I've stumbled over the fact that the current DI registry system is this close to being able to be straightforwardly type-check-able for TS consumers… and the change that would be required would IMO make for a nicer API (and cleaner internals!—at least once through the deprecation cycle) than the current one.

The current API for registrations and lookups is:

interface Owner { // ApplicationInstance, EngineInstance, ContainerProxyMixin...
  register(
    fullName: string,
    factory: any,
    options?: { singleton?: boolean; instantiate?: boolean }
  ): any;

  lookup(
    fullName: string,
    options?: { singleton?: boolean }
  ): any;
}

Here, fullName is a string like '<bucket>:<itemName>'—for example, 'service:session'.

It would be extremely helpful for TypeScript consumers (but also, in my opinion, nicer for everyone) if the registration and lookup API were:

interface Owner { // ApplicationInstance, EngineInstance, ContainerProxyMixin...
  register(
    type: string,
    name: string,
    factory: any,
    options?: { singleton?: boolean; instantiate?: boolean }
  ): any;

  lookup(
    type: string,
    name: string,
    options?: { singleton?: boolean }
  ): any;
}

The resulting API usage would look like this for lookup (and similar for register:

- const session = getOwner(this).lookup('service:session');
+ const session = getOwner(this).lookup('service', 'session');

For our purposes within the TS community, this would allow us to actually guarantee—with only tiny changes to work we've already done—that when a user looks up a service by doing getOwner(this).lookup('service', 'session') would safely give them a Sessioninstance, and we could also type-check the string passed.

Outside the TS community, I can say that I know the '<bucket>:<name>' pattern confused me early on, and that I simply find two separate arguments to be much clearer.

I'm happy to write an RFC that proposes this. If I did, I would propose starting by adding the new design within the current implementation, deprecating the single-string version of lookup and register at the same time, and then removing the single-string version at 4.0.

[rwjblue]

Seems good generally, but the RFCs changes would have to apply to all APIs that currently support type:name. This would include at least:

• ApplicationInstance ("owner") (a few additional APIs: factoryFor, inject, lookup, register, registerOptions, registerOptionsForType, ....)
• Resolver.prototype.resolve - This one is somewhat harder to deal with compatibly (because it is a third party provided API, though probably 99% of folks use ember-resolver)

I'd also probably use type instead of bucket.

[chriskrycho]

Yep, I figured that would be the case! I'd be happy to nail down all of those pieces. (Should have mentioned in the initial writeup that I knew there were others besides the ones I mentioned.)

With regard to Resolver.prototype.resolve—interesting. I knew we supported third-party resolvers, had never dug into how we do it. If I'm following correctly, the design there is that the third-party resolver just replaces Resolver.prototype.resolve? 🤔

Edit: Oh, and yes, type is much better than bucket; it was just the first thing I thought of at 6:30am when I started drafting this. 😂 Edit again: I just updated the sample signatures in the issue to match this suggestion.

[chriskrycho]

RFC opened as #585!