Make construction method for objects in `Injector` explicit

Previously a heuristic based on function naming was used to determine
whether `Injector` should construct objects with `new` or not. This is
brittle because compiler transforms may alter function names (eg. terser
removes them by default).

This commit re-works the API so that the caller explicitly indicates how
the object should be created (the "provider" to use). As a convenience
if a function is passed it is assumed to be a class.

```
// Register a class.
register("anObject", SomeClass);
register("anObject", { class: SomeClass });

// Register a factory.
register("anObject", { factory: makeMeAnObject });

// Register a plain value.
register("anObject", { value: "foobar" });
```

src/shared/injector.js

 /* global Map */
 
-function isClass(functionOrClass) {
-  return functionOrClass.name.match(/^[A-Z]/);
+/**
+ * @typedef Provider
+ * @prop {any} [value] - The value for the object
+ * @prop {Function} [class] - A class that should be instantiated to create the object
+ * @prop {Function} [factory] - Function that should be called to create the object
+ */
+
+/**
+ * @param {Provider} provider
+ */
+function isValidProvider(provider) {
+  if (typeof provider !== 'object' || provider === null) {
+    return false;
+  }
+
+  return (
+    'value' in provider ||
+    typeof provider.class === 'function' ||
+    typeof provider.factory === 'function'
+  );
 }
 
 /**
  * `Injector` is a dependency injection container.
  *
  * It provides a convenient way to instantiate a set of named objects with
- * dependencies. Objects are constructed using a factory function or
- * class constructor. The factory function or constructor may have dependencies
+ * dependencies. Objects are constructed using a _provider_, which can be a
+ * factory function, class constructor or value.
+ *
+ * If the provider is a factory function or constructor it may have dependencies
  * which are indicated by a `$inject` property on the function/class which
  * is a list of the names of the dependencies. The `$inject` property can be
  * added manually or by a compiler plugin (eg. `babel-plugin-angularjs-annotate`).
  *
- * To construct an object, call the `register` method with the name and factory
- * function/class for the object and each of its dependencies, and then call
- * the `get` method to construct or return the existing object with a given name,
- * along with all of its dependencies.
+ * To construct an object, call the `register` method with the name and provider
+ * for the object and each of its dependencies, and then call
+ * the `get` method to construct the object and its dependencies return it.
  */
 export class Injector {
   constructor() {
-    // Map of name to factory function that creates an instance or class used
-    // as prototype for instance.
-    this._factories = new Map();
+    // Map of name to object specifying how to create/provide that object.
+    this._providers = new Map();
 
     // Map of name to existing instance.
     this._instances = new Map();
@@ -44,12 +62,17 @@ export class Injector {
       return this._instances.get(name);
     }
 
-    const factory = this._factories.get(name);
+    const provider = this._providers.get(name);
 
-    if (!factory) {
+    if (!provider) {
       throw new Error(`"${name}" is not registered`);
     }
 
+    if ('value' in provider) {
+      this._instances.set(name, provider.value);
+      return provider.value;
+    }
+
     if (this._constructing.has(name)) {
       throw new Error(
         `Encountered a circular dependency when constructing "${name}"`
@@ -59,7 +82,10 @@ export class Injector {
     this._constructing.add(name);
     try {
       const resolvedDependencies = [];
-      const dependencies = factory.$inject || [];
+      const dependencies =
+        ('class' in provider && provider.class.$inject) ||
+        ('factory' in provider && provider.factory.$inject) ||
+        [];
 
       for (const dependency of dependencies) {
         try {
@@ -74,10 +100,11 @@ export class Injector {
       }
 
       let instance;
-      if (isClass(factory)) {
+      if (provider.class) {
         // eslint-disable-next-line new-cap
-        instance = new factory(...resolvedDependencies);
+        instance = new provider.class(...resolvedDependencies);
       } else {
+        const factory = provider.factory;
         instance = factory(...resolvedDependencies);
       }
       this._instances.set(name, instance);
@@ -89,20 +116,24 @@ export class Injector {
   }
 
   /**
-   * Register a factory with a given name.
+   * Register a provider for an object in the container.
    *
-   * If `factory`'s name starts with an upper-case letter it is treated as a
-   * class. If it starts with a lower-case letter it is treated as a factory
-   * function, which may return any type of value.
+   * If `provider` is a function, it is treated like a class. In other words
+   * `register(name, SomeClass)` is the same as `register(name, { class: SomeClass })`.
    *
    * @param {string} name - Name of object
-   * @param {() => any} factory -
-   *   A function that constructs the service, or a class that will be instantiated
-   *   when the object is requested.
+   * @param {Function|Provider} provider -
+   *   The class or other provider to use to create the object.
    * @return {this}
    */
-  register(name, factory) {
-    this._factories.set(name, factory);
+  register(name, provider) {
+    if (typeof provider === 'function') {
+      provider = { class: provider };
+    } else if (!isValidProvider(provider)) {
+      throw new Error(`Invalid provider for "${name}"`);
+    }
+
+    this._providers.set(name, provider);
     return this;
   }
 }

src/shared/test/injector-test.js

@@ -2,25 +2,41 @@ import { Injector } from '../injector';
 
 describe('Injector', () => {
   describe('#get', () => {
-    it('calls a non-class factory as a function to create instance', () => {
-      const instance = {};
-      const factory = sinon.stub().returns(instance);
+    it('calls a "factory" provider as a function to create the object', () => {
+      const instance = 'aValue';
+      const factory = sinon.stub().callsFake(function() {
+        assert.isUndefined(this);
+        return instance;
+      });
 
       const container = new Injector();
-      container.register('service', factory);
+      container.register('service', { factory });
       const constructed = container.get('service');
 
       assert.equal(constructed, instance);
     });
 
-    it('calls a class factory with `new` to create instance', () => {
+    it('calls a "class" provider with `new` to create instance', () => {
       class Foo {}
 
       const container = new Injector();
       container.register('foo', Foo);
+      container.register('foo2', { class: Foo });
 
       const constructed = container.get('foo');
       assert.instanceOf(constructed, Foo);
+
+      const constructed2 = container.get('foo2');
+      assert.instanceOf(constructed2, Foo);
+    });
+
+    it('uses the value of a "value" provider as the instance', () => {
+      const instance = {};
+
+      const container = new Injector();
+      container.register('anObject', { value: instance });
+
+      assert.equal(container.get('anObject'), instance);
     });
 
     it('returns the existing instance if already constructed', () => {
@@ -64,20 +80,16 @@ describe('Injector', () => {
         return factory;
       };
 
-      container.register('a', () => 'a');
-      container.register(
-        'b',
-        addDeps(a => a + 'b', ['a'])
-      );
-      container.register(
-        'c',
-        addDeps((b, a) => b + 'c' + a, ['b', 'a'])
-      );
+      container.register('a', { value: 'a' });
+      container.register('b', { factory: addDeps(a => a + 'b', ['a']) });
+      container.register('c', {
+        factory: addDeps((b, a) => b + 'c' + a, ['b', 'a']),
+      });
 
       assert.equal(container.get('c'), 'abca');
     });
 
-    it('throws an error if factory is not registered', () => {
+    it('throws an error if provider is not registered for name', () => {
       const container = new Injector();
       assert.throws(() => {
         container.get('invalid');
@@ -131,5 +143,14 @@ describe('Injector', () => {
         container
       );
     });
+
+    [{}, 'invalid', true, null, undefined].forEach(invalidProvider => {
+      it('throws an error if the provider is not valid', () => {
+        const container = new Injector();
+        assert.throws(() => {
+          container.register('foo', invalidProvider);
+        }, `Invalid provider for "foo"`);
+      });
+    });
   });
 });

src/sidebar/index.js

@@ -246,16 +246,16 @@ function startAngularApp(config) {
   // nb. In many cases these can be replaced by direct imports in the services
   // that use them, since they don't depend on instances of other services.
   container
-    .register('$window', () => window)
-    .register('Discovery', () => Discovery)
-    .register('OAuthClient', () => OAuthClient)
-    .register('VirtualThreadList', () => VirtualThreadList)
-    .register('isSidebar', () => isSidebar)
-    .register('random', () => random)
-    .register('serviceConfig', () => serviceConfig)
-    .register('settings', () => config)
-    .register('time', () => time)
-    .register('urlEncodeFilter', () => urlEncodeFilter);
+    .register('$window', { value: window })
+    .register('Discovery', { value: Discovery })
+    .register('OAuthClient', { value: OAuthClient })
+    .register('VirtualThreadList', { value: VirtualThreadList })
+    .register('isSidebar', { value: isSidebar })
+    .register('random', { value: random })
+    .register('serviceConfig', { value: serviceConfig })
+    .register('settings', { value: config })
+    .register('time', { value: time })
+    .register('urlEncodeFilter', { value: urlEncodeFilter });
 
   // Register services which only Angular can construct, once Angular has
   // constructed them.
@@ -263,8 +263,8 @@ function startAngularApp(config) {
   // @ngInject
   function registerAngularServices($rootScope, toastr) {
     container
-      .register('toastr', () => toastr)
-      .register('$rootScope', () => $rootScope);
+      .register('toastr', { value: toastr })
+      .register('$rootScope', { value: $rootScope });
   }
 
   angular