refactor: switch to new approach without ReflectiveInjector

Author: crisbeto

src/e2e-app/block-scroll-strategy/block-scroll-strategy-e2e.ts

@@ -1,5 +1,5 @@
 import {Component} from '@angular/core';
-import {BlockScrollStrategy, ViewportRuler} from '@angular/material';
+import {ScrollStrategyOptions, ScrollStrategy} from '@angular/material';
 
 @Component({
   moduleId: module.id,
@@ -8,6 +8,6 @@ import {BlockScrollStrategy, ViewportRuler} from '@angular/material';
   styleUrls: ['block-scroll-strategy-e2e.css'],
 })
 export class BlockScrollStrategyE2E {
-  constructor(private _viewportRuler: ViewportRuler) { }
-  scrollStrategy = new BlockScrollStrategy(this._viewportRuler);
+  constructor(private _scrollStrategyOptions: ScrollStrategyOptions) { }
+  scrollStrategy: ScrollStrategy = this._scrollStrategyOptions.get('block');
 }

src/lib/core/overlay/overlay-ref.ts

@@ -23,8 +23,11 @@ export class OverlayRef implements PortalHost {
       private _scrollStrategy: ScrollStrategy,
       private _ngZone: NgZone) {
 
-    _scrollStrategy.attach(this,
-        typeof _state.scrollStrategy === 'string' ? null : _state.scrollStrategy.config);
+    let scrollStrategyConfig = typeof _state.scrollStrategy === 'string' ?
+        null :
+        _state.scrollStrategy.config;
+
+    _scrollStrategy.attach(this, scrollStrategyConfig);
   }
 
   /** The overlay's HTML element */

src/lib/core/overlay/overlay.spec.ts

@@ -8,7 +8,8 @@ import {OverlayState} from './overlay-state';
 import {OverlayRef} from './overlay-ref';
 import {PositionStrategy} from './position/position-strategy';
 import {OverlayModule} from './overlay-directives';
-import {ScrollStrategy} from './scroll/scroll-strategy';
+import {ScrollStrategy, ScrollStrategyOptions, ScrollDispatcher} from './scroll/index';
+import {ViewportRuler} from './position/viewport-ruler';
 
 
 describe('Overlay', () => {
@@ -22,10 +23,20 @@ describe('Overlay', () => {
     TestBed.configureTestingModule({
       imports: [OverlayModule, PortalModule, OverlayTestModule],
       providers: [
-        {provide: OverlayContainer, useFactory: () => {
-          overlayContainerElement = document.createElement('div');
-          return {getContainerElement: () => overlayContainerElement};
-        }}
+        ScrollDispatcher,
+        ViewportRuler,
+        {
+          provide: ScrollStrategyOptions,
+          useClass: ScrollStrategyOptionsOverride,
+          deps: [ScrollDispatcher, ViewportRuler]
+        },
+        {
+          provide: OverlayContainer,
+          useFactory: () => {
+            overlayContainerElement = document.createElement('div');
+            return {getContainerElement: () => overlayContainerElement};
+          }
+        }
       ]
     }).compileComponents();
   }));
@@ -337,51 +348,30 @@ describe('Overlay', () => {
   describe('scroll strategy', () => {
     let fakeScrollStrategy: FakeScrollStrategy;
     let config: OverlayState;
-
-    class FakeScrollStrategy implements ScrollStrategy {
-      isEnabled = false;
-      overlayRef: OverlayRef;
-
-      constructor() {
-        fakeScrollStrategy = this;
-      }
-
-      attach(overlayRef: OverlayRef) {
-        this.overlayRef = overlayRef;
-      }
-
-      enable() {
-        this.isEnabled = true;
-      }
-
-      disable() {
-        this.isEnabled = false;
-      }
-    }
+    let overlayRef: OverlayRef;
 
     beforeEach(() => {
+    });
+
+    beforeEach(inject([ScrollStrategyOptions], (scrollOptions: ScrollStrategyOptionsOverride) => {
       config = new OverlayState();
-      overlay.registerScrollStrategy('fake', FakeScrollStrategy);
       config.scrollStrategy = 'fake';
-    });
+      overlayRef = overlay.create(config);
+      fakeScrollStrategy =
+          scrollOptions.instances[scrollOptions.instances.length - 1] as FakeScrollStrategy;
+    }));
 
     it('should attach the overlay ref to the scroll strategy', () => {
-      let overlayRef = overlay.create(config);
-
       expect(fakeScrollStrategy.overlayRef).toBe(overlayRef,
           'Expected scroll strategy to have been attached to the current overlay ref.');
     });
 
     it('should enable the scroll strategy when the overlay is attached', () => {
-      let overlayRef = overlay.create(config);
-
       overlayRef.attach(componentPortal);
       expect(fakeScrollStrategy.isEnabled).toBe(true, 'Expected scroll strategy to be enabled.');
     });
 
     it('should disable the scroll strategy once the overlay is detached', () => {
-      let overlayRef = overlay.create(config);
-
       overlayRef.attach(componentPortal);
       expect(fakeScrollStrategy.isEnabled).toBe(true, 'Expected scroll strategy to be enabled.');
 
@@ -390,8 +380,6 @@ describe('Overlay', () => {
     });
 
     it('should disable the scroll strategy when the overlay is destroyed', () => {
-      let overlayRef = overlay.create(config);
-
       overlayRef.dispose();
       expect(fakeScrollStrategy.isEnabled).toBe(false, 'Expected scroll strategy to be disabled.');
     });
@@ -469,3 +457,37 @@ class FakePositionStrategy implements PositionStrategy {
 
   dispose() {}
 }
+
+
+class FakeScrollStrategy implements ScrollStrategy {
+  isEnabled = false;
+  overlayRef: OverlayRef;
+
+  attach(overlayRef: OverlayRef) {
+    this.overlayRef = overlayRef;
+  }
+
+  enable() {
+    this.isEnabled = true;
+  }
+
+  disable() {
+    this.isEnabled = false;
+  }
+}
+
+
+class ScrollStrategyOptionsOverride extends ScrollStrategyOptions {
+  constructor(scrollDispatcher: ScrollDispatcher, viewportRuler: ViewportRuler) {
+    super(scrollDispatcher, viewportRuler);
+  }
+
+  // used for accessing the current instance in unit tests.
+  public instances: ScrollStrategy[] = [];
+
+  get(strategy: string): ScrollStrategy {
+    let instance = strategy === 'fake' ? new FakeScrollStrategy() : super.get(strategy);
+    this.instances.push(instance);
+    return instance;
+  }
+}

src/lib/core/overlay/overlay.ts

@@ -5,19 +5,14 @@ import {
   Injector,
   NgZone,
   Provider,
-  ReflectiveInjector,
 } from '@angular/core';
 import {OverlayState} from './overlay-state';
 import {DomPortalHost} from '../portal/dom-portal-host';
 import {OverlayRef} from './overlay-ref';
 import {OverlayPositionBuilder} from './position/overlay-position-builder';
 import {VIEWPORT_RULER_PROVIDER} from './position/viewport-ruler';
 import {OverlayContainer, OVERLAY_CONTAINER_PROVIDER} from './overlay-container';
-import {ScrollStrategy} from './scroll/scroll-strategy';
-import {RepositionScrollStrategy} from './scroll/reposition-scroll-strategy';
-import {BlockScrollStrategy} from './scroll/block-scroll-strategy';
-import {CloseScrollStrategy} from './scroll/close-scroll-strategy';
-import {NoopScrollStrategy} from './scroll/noop-scroll-strategy';
+import {ScrollStrategy, ScrollStrategyOptions} from './scroll/index';
 
 
 /** Next overlay unique ID. */
@@ -37,19 +32,10 @@ let defaultState = new OverlayState();
  */
 @Injectable()
 export class Overlay {
-  // Create a child ReflectiveInjector, allowing us to instantiate scroll
-  // strategies without going throught the injector cache.
-  private _reflectiveInjector = ReflectiveInjector.resolveAndCreate([], this._injector);
-  private _scrollStrategies = {
-    reposition: RepositionScrollStrategy,
-    block: BlockScrollStrategy,
-    close: CloseScrollStrategy,
-    noop: NoopScrollStrategy
-  };
-
   constructor(private _overlayContainer: OverlayContainer,
               private _componentFactoryResolver: ComponentFactoryResolver,
               private _positionBuilder: OverlayPositionBuilder,
+              private _scrollStrategyOptions: ScrollStrategyOptions,
               private _appRef: ApplicationRef,
               private _injector: Injector,
               private _ngZone: NgZone) { }
@@ -71,17 +57,6 @@ export class Overlay {
     return this._positionBuilder;
   }
 
-  /**
-   * Registers a scroll strategy to be available for use when creating an overlay.
-   * @param name Name of the scroll strategy.
-   * @param constructor Class to be used to instantiate the scroll strategy.
-   */
-  registerScrollStrategy(name: string, constructor: Function): void {
-    if (name && constructor) {
-      this._scrollStrategies[name] = constructor;
-    }
-  }
-
   /**
    * Creates the DOM element for an overlay and appends it to the overlay container.
    * @returns Newly-created pane element
@@ -111,29 +86,14 @@ export class Overlay {
    * @param state
    */
   private _createOverlayRef(pane: HTMLElement, state: OverlayState): OverlayRef {
+    let scrollStrategyName = typeof state.scrollStrategy === 'string' ?
+      state.scrollStrategy :
+      state.scrollStrategy.name;
+
+    let scrollStrategy = this._scrollStrategyOptions.get(scrollStrategyName);
     let portalHost = this._createPortalHost(pane);
-    let scrollStrategy = this._createScrollStrategy(state);
     return new OverlayRef(portalHost, pane, state, scrollStrategy, this._ngZone);
   }
-
-  /**
-   * Resolves the scroll strategy of an overlay state.
-   * @param state State for which to resolve the scroll strategy.
-   */
-  private _createScrollStrategy(state: OverlayState): ScrollStrategy {
-    let strategyName = typeof state.scrollStrategy === 'string' ?
-        state.scrollStrategy :
-        state.scrollStrategy.name;
-
-    if (!this._scrollStrategies.hasOwnProperty(strategyName)) {
-      throw new Error(`Unsupported scroll strategy "${strategyName}". The available scroll ` +
-                      `strategies are ${Object.keys(this._scrollStrategies).join(', ')}.`);
-    }
-
-    // Note that we use `resolveAndInstantiate` which will instantiate
-    // the scroll strategy without putting it in the injector cache.
-    return this._reflectiveInjector.resolveAndInstantiate(this._scrollStrategies[strategyName]);
-  }
 }
 
 /** Providers for Overlay and its related injectables. */

src/lib/core/overlay/scroll/block-scroll-strategy.spec.ts

@@ -4,7 +4,6 @@ import {
   ComponentPortal,
   OverlayModule,
   PortalModule,
-  BlockScrollStrategy,
   Platform,
   ViewportRuler,
   OverlayState,

src/lib/core/overlay/scroll/block-scroll-strategy.ts

@@ -1,11 +1,9 @@
-import {Injectable} from '@angular/core';
 import {ScrollStrategy} from './scroll-strategy';
 import {ViewportRuler} from '../position/viewport-ruler';
 
 /**
  * Strategy that will prevent the user from scrolling while the overlay is visible.
  */
-@Injectable()
 export class BlockScrollStrategy implements ScrollStrategy {
   private _previousHTMLStyles = { top: null, left: null };
   private _previousScrollPosition: { top: number, left: number };

src/lib/core/overlay/scroll/close-scroll-strategy.spec.ts

@@ -10,7 +10,6 @@ import {
   OverlayModule,
   ScrollStrategy,
   ScrollDispatcher,
-  CloseScrollStrategy,
 } from '../../core';
 
 

src/lib/core/overlay/scroll/close-scroll-strategy.ts

@@ -1,4 +1,3 @@
-import {Injectable} from '@angular/core';
 import {ScrollStrategy, getMdScrollStrategyAlreadyAttachedError} from './scroll-strategy';
 import {OverlayRef} from '../overlay-ref';
 import {Subscription} from 'rxjs/Subscription';
@@ -8,7 +7,6 @@ import {ScrollDispatcher} from './scroll-dispatcher';
 /**
  * Strategy that will close the overlay as soon as the user starts scrolling.
  */
-@Injectable()
 export class CloseScrollStrategy implements ScrollStrategy {
   private _scrollSubscription: Subscription|null = null;
   private _overlayRef: OverlayRef;

src/lib/core/overlay/scroll/index.ts

@@ -2,12 +2,14 @@ import {NgModule} from '@angular/core';
 import {SCROLL_DISPATCHER_PROVIDER} from './scroll-dispatcher';
 import {Scrollable} from './scrollable';
 import {PlatformModule} from '../../platform/index';
+import {ScrollStrategyOptions} from './scroll-strategy-options';
 
 export {Scrollable} from './scrollable';
 export {ScrollDispatcher} from './scroll-dispatcher';
 
 // Export pre-defined scroll strategies and interface to build custom ones.
 export {ScrollStrategy} from './scroll-strategy';
+export {ScrollStrategyOptions} from './scroll-strategy-options';
 export {RepositionScrollStrategy} from './reposition-scroll-strategy';
 export {CloseScrollStrategy} from './close-scroll-strategy';
 export {NoopScrollStrategy} from './noop-scroll-strategy';
@@ -17,6 +19,6 @@ export {BlockScrollStrategy} from './block-scroll-strategy';
   imports: [PlatformModule],
   exports: [Scrollable],
   declarations: [Scrollable],
-  providers: [SCROLL_DISPATCHER_PROVIDER],
+  providers: [SCROLL_DISPATCHER_PROVIDER, ScrollStrategyOptions],
 })
 export class ScrollDispatchModule { }

src/lib/core/overlay/scroll/noop-scroll-strategy.ts

@@ -1,10 +1,8 @@
-import {Injectable} from '@angular/core';
 import {ScrollStrategy} from './scroll-strategy';
 
 /**
  * Scroll strategy that doesn't do anything.
  */
-@Injectable()
 export class NoopScrollStrategy implements ScrollStrategy {
   enable() { }
   disable() { }

src/lib/core/overlay/scroll/reposition-scroll-strategy.spec.ts

@@ -10,7 +10,6 @@ import {
   OverlayModule,
   ScrollStrategy,
   ScrollDispatcher,
-  RepositionScrollStrategy,
 } from '../../core';
 
 

src/lib/core/overlay/scroll/reposition-scroll-strategy.ts

@@ -1,4 +1,3 @@
-import {Injectable} from '@angular/core';
 import {Subscription} from 'rxjs/Subscription';
 import {ScrollStrategy, getMdScrollStrategyAlreadyAttachedError} from './scroll-strategy';
 import {OverlayRef} from '../overlay-ref';
@@ -14,7 +13,6 @@ export interface RepositionScrollStrategyConfig {
 /**
  * Strategy that will update the element position as the user is scrolling.
  */
-@Injectable()
 export class RepositionScrollStrategy implements ScrollStrategy {
   private _scrollSubscription: Subscription|null = null;
   private _overlayRef: OverlayRef;

src/lib/core/overlay/scroll/scroll-strategy-options.ts

@@ -0,0 +1,31 @@
+import {Injectable} from '@angular/core';
+import {ScrollStrategy} from './scroll-strategy';
+import {RepositionScrollStrategy} from './reposition-scroll-strategy';
+import {CloseScrollStrategy} from './close-scroll-strategy';
+import {NoopScrollStrategy} from './noop-scroll-strategy';
+import {BlockScrollStrategy} from './block-scroll-strategy';
+import {ScrollDispatcher} from './scroll-dispatcher';
+import {ViewportRuler} from '../position/viewport-ruler';
+
+
+/**
+ * Factory that instantiates scroll strategies. Provides the built-in `reposition`, `close`,
+ * `noop` and `block` strategies by default.
+ */
+@Injectable()
+export class ScrollStrategyOptions {
+  constructor(
+    private _scrollDispatcher: ScrollDispatcher,
+    private _viewportRuler: ViewportRuler) { }
+
+  get(strategy: string): ScrollStrategy {
+    switch (strategy) {
+      case 'reposition': return new RepositionScrollStrategy(this._scrollDispatcher);
+      case 'close': return new CloseScrollStrategy(this._scrollDispatcher);
+      case 'noop': return new NoopScrollStrategy();
+      case 'block': return new BlockScrollStrategy(this._viewportRuler);
+    }
+
+    throw new Error(`Unsupported scroll strategy "${strategy}".`);
+  }
+}