Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

di: Allow arbitrary objects as identifiers for services #10347

Open
ewinslow opened this issue Dec 5, 2014 · 8 comments · May be fixed by #14998
Open

di: Allow arbitrary objects as identifiers for services #10347

ewinslow opened this issue Dec 5, 2014 · 8 comments · May be fixed by #14998

Comments

@ewinslow
Copy link
Contributor

ewinslow commented Dec 5, 2014

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
Copy link
Contributor

caitp commented Dec 5, 2014

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
Copy link
Contributor Author

ewinslow commented Dec 7, 2014

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
Copy link
Contributor

caitp commented Dec 7, 2014

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
Copy link
Contributor Author

ewinslow commented Dec 7, 2014

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
Copy link
Contributor

caitp commented Dec 7, 2014

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
Copy link

@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
Copy link
Contributor Author

@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
Copy link
Contributor

+1

gkalpak added a commit to gkalpak/angular.js that referenced this issue Aug 5, 2016
Previously, only strings could be used as identifiers for Angular services. This commit adds support
for using any value as identifier for an Angular service (e.g. used with `.provider()`,
`.factory()`, `.service()`, `.value()`, `.constant()`, `.decorator()`).

Identifiers for directives and filters are still restricted to string values, since they need to be
references in HTML (text).

--
As usual, if a service has a string ID, its provider's ID is constructed by appending `Provider` to
the service ID. For services with non-string IDs, their providers have the exact same ID (it is the
context that determines if a service or a provider should be injected).

E.g.:
```js
var bar = {};

angular.
  module('myModule', []).

  provider('foo' /* string ID     */, {$get: function () { return 'FOO'; }}).
  provider( bar  /* non-string ID */, {$get: function () { return 'BAR'; }}).

  config(['fooProvider', function (fooProvider) {
    // `foo` provider injected (because we are in config block)
  }]).
  run(['foo', function (foo) {
    // `foo` service injected (because we are in run block)
  }]).

  config([bar /* same ID */, function (barProvider) {
    // `bar` provider injected (because we are in config block)
  }]).
  run([bar /* same ID */, function (bar) {
    // `bar` service injected (because we are in run block)
  }]);
```

--
This change is backwards compatible (afaict).

Fixes angular#10347
gkalpak added a commit to gkalpak/angular.js that referenced this issue Aug 6, 2016
Previously, only strings could be used as identifiers for Angular services. This commit adds support
for using any value as identifier for an Angular service (e.g. used with `.provider()`,
`.factory()`, `.service()`, `.value()`, `.constant()`, `.decorator()`).

Identifiers for directives and filters are still restricted to string values, since they need to be
references in HTML (text).

--
As usual, if a service has a string ID, its provider's ID is constructed by appending `Provider` to
the service ID. For services with non-string IDs, their providers have the exact same ID (it is the
context that determines if a service or a provider should be injected).

E.g.:
```js
var bar = {};

angular.
  module('myModule', []).

  provider('foo' /* string ID     */, {$get: function () { return 'FOO'; }}).
  provider( bar  /* non-string ID */, {$get: function () { return 'BAR'; }}).

  config(['fooProvider', function (fooProvider) {
    // `foo` provider injected (because we are in config block)
  }]).
  run(['foo', function (foo) {
    // `foo` service injected (because we are in run block)
  }]).

  config([bar /* same ID */, function (barProvider) {
    // `bar` provider injected (because we are in config block)
  }]).
  run([bar /* same ID */, function (bar) {
    // `bar` service injected (because we are in run block)
  }]);
```

--
This change is backwards compatible (afaict).

Fixes angular#10347
gkalpak added a commit to gkalpak/angular.js that referenced this issue Aug 6, 2016
Previously, only strings could be used as identifiers for Angular services. This commit adds support
for using any value as identifier for an Angular service (e.g. used with `.provider()`,
`.factory()`, `.service()`, `.value()`, `.constant()`, `.decorator()`).

Identifiers for directives and filters are still restricted to string values, since they need to be
referenced in HTML (text).

--
For services with string IDs, the corresponding provider ID is constructed by appending `Provider`
to the service ID. For services with non-string IDs, the corresponding provider has the exact same
ID (it is the context that determines if a service or a provider should be injected).

E.g.:
```js
var bar = {};

angular.
  module('myModule', []).

  provider('foo' /* string ID     */, {$get: function () { return 'FOO'; }}).
  provider( bar  /* non-string ID */, {$get: function () { return 'BAR'; }}).

  config(['fooProvider', function (fooProvider) {
    // `foo` provider injected (because we are in config block)
  }]).
  run(['foo', function (foo) {
    // `foo` service injected (because we are in run block)
  }]).

  config([bar, function (barProvider) {
    // `bar` provider injected (because we are in config block)
    // (even though we used the same identifier (`bar`) that we will use in the run block)
  }]).
  run([bar, function (bar) {
    // `bar` service injected (because we are in run block)
  }]);
```

--
This change is backwards compatible (afaict).

Fixes angular#10347
gkalpak added a commit to gkalpak/angular.js that referenced this issue Aug 6, 2016
Previously, only strings could be used as identifiers for Angular services. This commit adds support
for using any value as identifier for an Angular service (e.g. used with `.provider()`,
`.factory()`, `.service()`, `.value()`, `.constant()`, `.decorator()`).

Among other things, non-string identifiers enable:
- Private services (without relying on naming conventions).
- Collision avoidance (without relying on custom prefixes and namespacing).
- Better toolability (e.g. autocomplete support for service identifiers).
- Better compression (i.e. strings can't be minified, non-string values could).

Identifiers for directives and filters are still restricted to string values, since they need to be
referenced in HTML (text).

--
For services with string IDs, the corresponding provider ID is constructed by appending `Provider`
to the service ID. For services with non-string IDs, the corresponding provider has the exact same
ID (it is the context that determines if a service or a provider should be injected).

E.g.:
```js
var bar = {};

angular.
  module('myModule', []).

  provider('foo' /* string ID     */, {$get: function () { return 'FOO'; }}).
  provider( bar  /* non-string ID */, {$get: function () { return 'BAR'; }}).

  config(['fooProvider', function (fooProvider) {
    // `foo` provider injected (because we are in config block)
  }]).
  run(['foo', function (foo) {
    // `foo` service injected (because we are in run block)
  }]).

  config([bar, function (barProvider) {
    // `bar` provider injected (because we are in config block)
    // (even though we used the same identifier (`bar`) that we will use in the run block)
  }]).
  run([bar, function (bar) {
    // `bar` service injected (because we are in run block)
  }]);
```

--
This change is backwards compatible (afaict).

Fixes angular#10347
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants