di: Allow arbitrary objects as identifiers for services

[ewinslow]

This could help us (start to) break away from the string-only ids for services in 1.x, without waiting for 2.0.

Example declaring an injection point

Foo.$inject = [Storage]; // instead of Foo.$inject = ['db'];
function Foo(db) {
 ... 
}

Example wiring up the implementation in an angular module:

// These are all meant to be equivalent
angular.module('foo', [])
  .value(Storage, new StorageImpl())
  .service(Storage, StorageImpl)
  .factory(Storage, function() { return new StorageImpl() })
  .provider(Storage, function() { this.$get = function() { return new StorageImpl() }; });

I am able to implement this in a library where I generate the string IDs for compatibility with Angular 1.3, but it feels pretty hacky and seems like something Angular core should take care of.

For reference, it ends up looking something like this:

Foo.$inject = inject(Storage, Other...);
function Foo(storage, other) {...}

angular.module('foo', [])
  .service(id(Storage), StorageImpl);

[caitp]

So, you can do this:

angular.module({
  name1, impl1,
  name2, impl2
});

So we can't just do a simple if (!isString(name)) intuitTheName() kinda thing --- it would have to look at the arguments count, and that's kind of not great.

But ignoring that, it's not really clear what we'd get out of it. Using WeakMaps can't be used because of the target browsers that we have to support, and our alternative solution using $$hashKey is problematic for tests, without extreme care taken

But even ignoring that, it's really just not clear that it's particularly useful --- what is better about using an object to identify a provider?

This seems like something we could think about for 1.4, but I wouldn't count on it being supported. A more interesting way to go might be getting di.js to work in angular 1.x instead.

[ewinslow]

Thanks so much for your reply! It means a lot that you bothered to look into this and take time to respond.

I haven't seen that syntax before. And it doesn't quite make sense to me. Do you have a typo in there maybe? In any case, if its part of the API then of course you have to support it, but I don't follow why it's relevant here. Apologies...

Did you mean angular.module('modName', []).service({name: Impl1})? That would obviously only work for string names still, which seems fine to me. You don't necessarily have to support non-string identifiers if people want that syntax.

Using WeakMaps can't be used because of the target browsers

Actually I don't think WeakMaps would be appropriate for this use-case. You'd want just a plain old Map, and that can be polyfilled. You'd have to use $$hashKey if you want to guarantee O(1) lookups, but you could also just write an O(n)-ish polyfill which defers to the faster native implementation in browsers that support it. I bet that would be fast enough for DI... I say "O(n)-ish" because you could keep lookups for string identifiers O(1). So n is really the number of non-string identifiers, not the total number of identifiers.

Instantiating a native Map looks like:

new Map([
 [key, value],
 [key2, value2],
]);

You could use a similar syntax if you want, but I don't think it's strictly necessary.

what is better about using an object to identify a provider?

Note by "object" I also mean "function", which would include the service implementation itself in many cases, so no need to think up a unique name, necessarily.

Main motivations:

• Minification of the identifiers
• Helps prevent accidental collisions between identifiers
• Migrating from this to 2.0 di seems like it would be a smaller ordeal vs migrating from current strings to 2.0 di.

There's also possibly some fancy advanced stuff this would allow like private services enforced by JS scoping rather than being "enforced" by string naming conventions and finger-crossing.

TBH I was a little taken aback by this objection. I've heard the Angular team rave publicly and repeatedly about the ability to use non-strings for DI annotations in 2.0. Is that not such a big selling point anymore to the team?

A more interesting way to go might be getting di.js to work in angular 1.x instead.

That would be great! I suspect that would be harder, though, and my proposal here seemed to me like the easiest way to get a good chunk of the benefits of di.js now + straightforward API compatibility with all Angular 1.x projects, without having to wait for di.js to fully bake. But hey, if you can make it work, more power to you.

I also could see how consolidating the internal implementation on di.js in the future once the details of how to make that work are determined. Baby steps :).

[caitp]

Actually I don't think WeakMaps would be appropriate for this use-case. You'd want just a plain old Map, and that can be polyfilled.

If an object is a key, it does need to be a Weak collection.

I haven't seen that syntax before. And it doesn't quite make sense to me. Do you have a typo in there maybe?

No.

angular.module("app", []).
  factory("fact1", someFn1).
  factory("fact2", someFn2);

Is equivalent to

angular.module("app", []).
  factory({
    "fact1": someFn1,
    "fact2": someFn2
  });

[ewinslow]

If an object is a key, it does need to be a Weak collection.

I really apologize but I'm not seeing why this is the case. Plain Map supports object/function keys too... From MDN:

The Map object is a simple key/value map. Any value (both objects and primitive values) may be used as either a key or a value.

Why would you want random garbage collection (_Weak_Map) of the DI configuration? That seems like asking for bugs. But perhaps I haven't thought this through...

[caitp]

Plain Map supports object/function keys too...

Yes --- it turns out it does. There was an older implementation bug which made me believe this was not the case. However, we still wouldn't use object keys in a Map anyways.

Why would you want random garbage collection (WeakMap) of the DI configuration? That seems like asking for bugs. But perhaps I haven't thought this through

You'd have to be very careful to avoid losing references when you still needed them. But it's besides the point because n'either case is going to be supported

[frfancha]

@ewinslow You say: 'Note by "object" I also mean "function", which would include the service implementation itself in many cases, so no need to think up a unique name, necessarily.' Sorry if I misunderstand something, but it seems a non-sense to me: if you use the service implementation to identify it in the declaration of your dependencies, then the full DI mecanism is just useless: you could as well completely ignore it and use the service implementation directly. Or do you mean something else?

[ewinslow]

@frfancha, that's a possible simplification, yes, but you could be injecting an "interface" (closure compiler supports these, for example). Then you need to bind that to an implementation. It just compiles down to an empty function in production, ultimately.

[mikehaas763]

+1