Merge pull request #1763 from hypothesis/dependency-injector-v2

Implement container to replace AngularJS's dependency injector for services (v2)

src/shared/injector.js

+/* global Map */
+
+/**
+ * @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 _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 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 object specifying how to create/provide that object.
+    this._providers = new Map();
+
+    // Map of name to existing instance.
+    this._instances = new Map();
+
+    // Set of instances already being constructed. Used to detect circular
+    // dependencies.
+    this._constructing = new Set();
+  }
+
+  /**
+   * Construct or return the existing instance of an object with a given `name`
+   *
+   * @param {string} name - Name of object to construct
+   * @return {any} - The constructed object
+   */
+  get(name) {
+    if (this._instances.has(name)) {
+      return this._instances.get(name);
+    }
+
+    const provider = this._providers.get(name);
+
+    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}"`
+      );
+    }
+
+    this._constructing.add(name);
+    try {
+      const resolvedDependencies = [];
+      const dependencies =
+        ('class' in provider && provider.class.$inject) ||
+        ('factory' in provider && provider.factory.$inject) ||
+        [];
+
+      for (const dependency of dependencies) {
+        try {
+          resolvedDependencies.push(this.get(dependency));
+        } catch (e) {
+          const resolveErr = new Error(
+            `Failed to construct dependency "${dependency}" of "${name}": ${e.message}`
+          );
+          resolveErr.cause = e;
+          throw resolveErr;
+        }
+      }
+
+      let instance;
+      if (provider.class) {
+        // eslint-disable-next-line new-cap
+        instance = new provider.class(...resolvedDependencies);
+      } else {
+        const factory = provider.factory;
+        instance = factory(...resolvedDependencies);
+      }
+      this._instances.set(name, instance);
+
+      return instance;
+    } finally {
+      this._constructing.delete(name);
+    }
+  }
+
+  /**
+   * Register a provider for an object in the container.
+   *
+   * 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 {Function|Provider} provider -
+   *   The class or other provider to use to create the object.
+   * @return {this}
+   */
+  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

+import { Injector } from '../injector';
+
+describe('Injector', () => {
+  describe('#get', () => {
+    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 });
+      const constructed = container.get('service');
+
+      assert.equal(constructed, 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', () => {
+      const instance = {};
+      const factory = sinon.stub().returns(instance);
+
+      const container = new Injector();
+      container.register('service', factory);
+
+      container.get('service');
+      const constructed = container.get('service');
+
+      assert.equal(constructed, instance);
+      assert.calledOnce(factory);
+    });
+
+    it('resolves dependencies', () => {
+      const container = new Injector();
+
+      const foo = {};
+      const bar = {};
+
+      const fooFactory = sinon.stub().returns(foo);
+      const barFactory = sinon.stub().returns(bar);
+      const bazFactory = (foo, bar) => ({ foo, bar });
+      bazFactory.$inject = ['foo', 'bar'];
+
+      container.register('foo', fooFactory);
+      container.register('bar', barFactory);
+      container.register('baz', bazFactory);
+
+      const baz = container.get('baz');
+      assert.equal(baz.foo, foo);
+      assert.equal(baz.bar, bar);
+    });
+
+    it('resolves transitive dependencies', () => {
+      const container = new Injector();
+      const addDeps = (factory, dependencies) => {
+        factory.$inject = dependencies;
+        return factory;
+      };
+
+      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 provider is not registered for name', () => {
+      const container = new Injector();
+      assert.throws(() => {
+        container.get('invalid');
+      }, '"invalid" is not registered');
+    });
+
+    it('throws an error if dependency is not registered', () => {
+      const container = new Injector();
+      const fooFactory = () => 42;
+      fooFactory.$inject = ['bar'];
+
+      container.register('foo', fooFactory);
+
+      assert.throws(() => {
+        container.get('foo');
+      }, 'Failed to construct dependency "bar" of "foo"');
+    });
+
+    it('throws an error if a circular dependency is encountered', () => {
+      const container = new Injector();
+      const fooFactory = () => 42;
+      fooFactory.$inject = ['foo'];
+
+      container.register('foo', fooFactory);
+
+      let err;
+      try {
+        container.get('foo');
+      } catch (e) {
+        err = e;
+      }
+
+      assert.instanceOf(err, Error);
+      assert.equal(
+        err.toString(),
+        'Error: Failed to construct dependency "foo" of "foo": Encountered a circular dependency when constructing "foo"'
+      );
+      assert.instanceOf(err.cause, Error);
+      assert.equal(
+        err.cause.toString(),
+        'Error: Encountered a circular dependency when constructing "foo"'
+      );
+    });
+  });
+
+  describe('#register', () => {
+    it('returns container for chaining', () => {
+      const container = new Injector();
+      assert.equal(
+        container.register('foo', () => 42),
+        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

@@ -205,7 +205,68 @@ import * as random from './util/random';
 import * as time from './util/time';
 import VirtualThreadList from './virtual-thread-list';
 
+import { Injector } from '../shared/injector';
+
 function startAngularApp(config) {
+  // Create dependency injection container for services.
+  //
+  // This is a replacement for the use of Angular's dependency injection
+  // (including its `$injector` service) to construct services with dependencies.
+  const container = new Injector();
+
+  // Register services.
+  container
+    .register('analytics', analyticsService)
+    .register('annotationMapper', annotationMapperService)
+    .register('annotationsService', annotationsService)
+    .register('api', apiService)
+    .register('apiRoutes', apiRoutesService)
+    .register('auth', authService)
+    .register('bridge', bridgeService)
+    .register('features', featuresService)
+    .register('flash', flashService)
+    .register('frameSync', frameSyncService)
+    .register('groups', groupsService)
+    .register('localStorage', localStorageService)
+    .register('permissions', permissionsService)
+    .register('persistedDefaults', persistedDefaultsService)
+    .register('rootThread', rootThreadService)
+    .register('searchFilter', searchFilterService)
+    .register('serviceUrl', serviceUrlService)
+    .register('session', sessionService)
+    .register('streamer', streamerService)
+    .register('streamFilter', streamFilterService)
+    .register('tags', tagsService)
+    .register('unicode', unicodeService)
+    .register('viewFilter', viewFilterService)
+    .register('store', store);
+
+  // Register utility values/classes.
+  //
+  // 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', { 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.
+  //
+  // @ngInject
+  function registerAngularServices($rootScope, toastr) {
+    container
+      .register('toastr', { value: toastr })
+      .register('$rootScope', { value: $rootScope });
+  }
+
   angular
     .module('h', [angularRoute, angularToastr])
 
@@ -250,48 +311,53 @@ function startAngularApp(config) {
     .directive('hTooltip', hTooltipDirective)
     .directive('windowScroll', windowScrollDirective)
 
-    .service('analytics', analyticsService)
-    .service('annotationMapper', annotationMapperService)
-    .service('annotationsService', annotationsService)
-    .service('api', apiService)
-    .service('apiRoutes', apiRoutesService)
-    .service('auth', authService)
-    .service('bridge', bridgeService)
-    .service('features', featuresService)
-    .service('flash', flashService)
-    .service('frameSync', frameSyncService)
-    .service('groups', groupsService)
-    .service('localStorage', localStorageService)
-    .service('permissions', permissionsService)
-    .service('persistedDefaults', persistedDefaultsService)
-    .service('rootThread', rootThreadService)
-    .service('searchFilter', searchFilterService)
-    .service('serviceUrl', serviceUrlService)
-    .service('session', sessionService)
-    .service('streamer', streamerService)
-    .service('streamFilter', streamFilterService)
-    .service('tags', tagsService)
-    .service('unicode', unicodeService)
-    .service('viewFilter', viewFilterService)
+    // Register services, the store and utilities with Angular, so that
+    // Angular components can use them.
+    .service('analytics', () => container.get('analytics'))
+    .service('annotationMapper', () => container.get('annotationMapper'))
+    .service('annotationsService', () => container.get('annotationsService'))
+    .service('api', () => container.get('api'))
+    .service('apiRoutes', () => container.get('apiRoutes'))
+    .service('auth', () => container.get('auth'))
+    .service('bridge', () => container.get('bridge'))
+    .service('features', () => container.get('features'))
+    .service('flash', () => container.get('flash'))
+    .service('frameSync', () => container.get('frameSync'))
+    .service('groups', () => container.get('groups'))
+    .service('localStorage', () => container.get('localStorage'))
+    .service('permissions', () => container.get('permissions'))
+    .service('persistedDefaults', () => container.get('persistedDefaults'))
+    .service('rootThread', () => container.get('rootThread'))
+    .service('searchFilter', () => container.get('searchFilter'))
+    .service('serviceUrl', () => container.get('serviceUrl'))
+    .service('session', () => container.get('session'))
+    .service('streamer', () => container.get('streamer'))
+    .service('streamFilter', () => container.get('streamFilter'))
+    .service('tags', () => container.get('tags'))
+    .service('unicode', () => container.get('unicode'))
+    .service('viewFilter', () => container.get('viewFilter'))
 
     // Redux store
-    .service('store', store)
+    .service('store', () => container.get('store'))
 
     // Utilities
-    .value('Discovery', Discovery)
-    .value('OAuthClient', OAuthClient)
-    .value('VirtualThreadList', VirtualThreadList)
-    .value('isSidebar', isSidebar)
-    .value('random', random)
-    .value('serviceConfig', serviceConfig)
-    .value('settings', config)
-    .value('time', time)
-    .value('urlEncodeFilter', urlEncodeFilter)
+    .value('Discovery', container.get('Discovery'))
+    .value('OAuthClient', container.get('OAuthClient'))
+    .value('VirtualThreadList', container.get('VirtualThreadList'))
+    .value('isSidebar', container.get('isSidebar'))
+    .value('random', container.get('random'))
+    .value('serviceConfig', container.get('serviceConfig'))
+    .value('settings', container.get('settings'))
+    .value('time', container.get('time'))
+    .value('urlEncodeFilter', container.get('urlEncodeFilter'))
 
     .config(configureLocation)
     .config(configureRoutes)
     .config(configureToastr)
 
+    // Make Angular built-ins available to services constructed by `container`.
+    .run(registerAngularServices)
+
     .run(persistDefaults)
     .run(sendPageView)
     .run(setupApi)

src/sidebar/services/view-filter.js

@@ -20,7 +20,7 @@ function displayName(ann) {
  * which do not match the filter are then hidden.
  */
 // @ngInject
-export default function viewFilter(unicode) {
+export default function ViewFilter(unicode) {
   /**
    * Normalize a field value or query term for comparison.
    */