refactor(google-maps): Removes reference to GoogleMap from MapMarker (#17222)

Get access to the GoogleMap parent component from a MapMarker through
the constructor instead of calling a "setMap" method on the marker from
the GoogleMap to prevent MapMarker being loaded in all maps.

Author: mbehrlich

src/google-maps/google-map/google-map.spec.ts

@@ -3,7 +3,6 @@ import {async, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 
 import {GoogleMapsModule} from '../google-maps-module';
-import {MapMarker} from '../map-marker/map-marker';
 import {
   createMapConstructorSpy,
   createMapSpy,
@@ -240,18 +239,6 @@ describe('GoogleMap', () => {
     expect(mapSpy.addListener).not.toHaveBeenCalledWith('tilt_changed', jasmine.any(Function));
     expect(mapSpy.addListener).not.toHaveBeenCalledWith('zoom_changed', jasmine.any(Function));
   });
-
-  it('calls setMap on child marker components', () => {
-    mapSpy = createMapSpy(DEFAULT_OPTIONS);
-    createMapConstructorSpy(mapSpy).and.callThrough();
-
-    const fixture = TestBed.createComponent(TestApp);
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    spyOn(markerComponent, '_setMap').and.callThrough();
-    fixture.detectChanges();
-
-    expect(markerComponent._setMap).toHaveBeenCalledWith(mapSpy);
-  });
 });
 
 @Component({
@@ -264,7 +251,6 @@ describe('GoogleMap', () => {
                          (mapClick)="handleClick($event)"
                          (centerChanged)="handleCenterChanged()"
                          (mapRightclick)="handleRightclick($event)">
-              <map-marker></map-marker>
             </google-map>`,
 })
 class TestApp {

src/google-maps/google-map/google-map.ts

@@ -7,25 +7,20 @@
  */
 
 import {
-  AfterContentInit,
   ChangeDetectionStrategy,
   Component,
-  ContentChildren,
   ElementRef,
   EventEmitter,
   Input,
   OnChanges,
   OnDestroy,
   OnInit,
   Output,
-  QueryList,
   ViewEncapsulation,
 } from '@angular/core';
 import {BehaviorSubject, combineLatest, Observable, Subject} from 'rxjs';
 import {map, shareReplay, take, takeUntil} from 'rxjs/operators';
 
-import {MapMarker} from '../map-marker/map-marker';
-
 interface GoogleMapsWindow extends Window {
   google?: typeof google;
 }
@@ -62,7 +57,7 @@ export const DEFAULT_WIDTH = '500px';
   template: '<div class="map-container"></div><ng-content></ng-content>',
   encapsulation: ViewEncapsulation.None,
 })
-export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy {
+export class GoogleMap implements OnChanges, OnInit, OnDestroy {
   @Input() height = DEFAULT_HEIGHT;
 
   @Input() width = DEFAULT_WIDTH;
@@ -188,8 +183,6 @@ export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy
    */
   @Output() zoomChanged = new EventEmitter<void>();
 
-  @ContentChildren(MapMarker) _markers: QueryList<MapMarker>;
-
   private _mapEl: HTMLElement;
   _googleMap!: UpdatedGoogleMap;
 
@@ -234,13 +227,6 @@ export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy
     this._watchForOptionsChanges(combinedOptionsChanges);
   }
 
-  ngAfterContentInit() {
-    for (const marker of this._markers.toArray()) {
-      marker._setMap(this._googleMap);
-    }
-    this._watchForMarkerChanges();
-  }
-
   ngOnDestroy() {
     this._destroy.next();
     this._destroy.complete();
@@ -472,14 +458,4 @@ export class GoogleMap implements OnChanges, OnInit, AfterContentInit, OnDestroy
           }));
     }
   }
-
-  private _watchForMarkerChanges() {
-    combineLatest(this._googleMapChanges, this._markers.changes)
-        .pipe(takeUntil(this._destroy))
-        .subscribe(([googleMap, markers]) => {
-          for (let marker of markers) {
-            marker._setMap(googleMap);
-          }
-        });
-  }
 }

src/google-maps/map-marker/map-marker.spec.ts

@@ -2,7 +2,10 @@ import {Component} from '@angular/core';
 import {async, TestBed} from '@angular/core/testing';
 import {By} from '@angular/platform-browser';
 
+import {DEFAULT_OPTIONS, UpdatedGoogleMap} from '../google-map/google-map';
 import {
+  createMapConstructorSpy,
+  createMapSpy,
   createMarkerConstructorSpy,
   createMarkerSpy,
   TestingWindow
@@ -12,6 +15,8 @@ import {GoogleMapsModule} from '../google-maps-module';
 import {DEFAULT_MARKER_OPTIONS, MapMarker} from './map-marker';
 
 describe('MapMarker', () => {
+  let mapSpy: jasmine.SpyObj<UpdatedGoogleMap>;
+
   beforeEach(async(() => {
     TestBed.configureTestingModule({
       imports: [GoogleMapsModule],
@@ -21,6 +26,9 @@ describe('MapMarker', () => {
 
   beforeEach(() => {
     TestBed.compileComponents();
+
+    mapSpy = createMapSpy(DEFAULT_OPTIONS);
+    createMapConstructorSpy(mapSpy).and.callThrough();
   });
 
   afterEach(() => {
@@ -33,28 +41,24 @@ describe('MapMarker', () => {
     const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();
 
     const fixture = TestBed.createComponent(TestApp);
-    const fakeMap = {} as unknown as google.maps.Map;
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
     fixture.detectChanges();
 
     expect(markerConstructorSpy).toHaveBeenCalledWith({
       ...DEFAULT_MARKER_OPTIONS,
       title: undefined,
       label: undefined,
       clickable: undefined,
-      map: fakeMap
+      map: mapSpy,
     });
   });
 
   it('sets marker inputs', () => {
-    const fakeMap = {} as unknown as google.maps.Map;
     const options: google.maps.MarkerOptions = {
       position: {lat: 3, lng: 5},
       title: 'marker title',
       label: 'marker label',
       clickable: false,
-      map: fakeMap,
+      map: mapSpy,
     };
     const markerSpy = createMarkerSpy(options);
     const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();
@@ -64,15 +68,12 @@ describe('MapMarker', () => {
     fixture.componentInstance.title = options.title;
     fixture.componentInstance.label = options.label;
     fixture.componentInstance.clickable = options.clickable;
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
     fixture.detectChanges();
 
     expect(markerConstructorSpy).toHaveBeenCalledWith(options);
   });
 
   it('sets marker options, ignoring map', () => {
-    const fakeMap = {} as unknown as google.maps.Map;
     const options: google.maps.MarkerOptions = {
       position: {lat: 3, lng: 5},
       title: 'marker title',
@@ -85,15 +86,12 @@ describe('MapMarker', () => {
 
     const fixture = TestBed.createComponent(TestApp);
     fixture.componentInstance.options = options;
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
     fixture.detectChanges();
 
-    expect(markerConstructorSpy).toHaveBeenCalledWith({...options, map: fakeMap});
+    expect(markerConstructorSpy).toHaveBeenCalledWith({...options, map: mapSpy});
   });
 
   it('gives precedence to specific inputs over options', () => {
-    const fakeMap = {} as unknown as google.maps.Map;
     const options: google.maps.MarkerOptions = {
       position: {lat: 3, lng: 5},
       title: 'marker title',
@@ -107,7 +105,7 @@ describe('MapMarker', () => {
       label: 'updated label',
       clickable: true,
       icon: 'icon name',
-      map: fakeMap,
+      map: mapSpy,
     };
     const markerSpy = createMarkerSpy(options);
     const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();
@@ -118,43 +116,17 @@ describe('MapMarker', () => {
     fixture.componentInstance.label = expectedOptions.label;
     fixture.componentInstance.clickable = expectedOptions.clickable;
     fixture.componentInstance.options = options;
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
     fixture.detectChanges();
 
     expect(markerConstructorSpy).toHaveBeenCalledWith(expectedOptions);
   });
 
-  it('sets the map on the marker only once', () => {
-    const markerSpy = createMarkerSpy(DEFAULT_MARKER_OPTIONS);
-    const markerConstructorSpy = createMarkerConstructorSpy(markerSpy).and.callThrough();
-
-    const fixture = TestBed.createComponent(TestApp);
-    const fakeMap = {} as unknown as google.maps.Map;
-    const fakeMap2 = {testValue: 'test'} as unknown as google.maps.Map;
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
-    markerComponent._setMap(fakeMap2);
-    fixture.detectChanges();
-
-    expect(markerConstructorSpy).toHaveBeenCalledWith({
-      ...DEFAULT_MARKER_OPTIONS,
-      title: undefined,
-      label: undefined,
-      clickable: undefined,
-      map: fakeMap
-    });
-    expect(markerSpy.setOptions).not.toHaveBeenCalled();
-  });
-
   it('exposes methods that provide information about the marker', () => {
     const markerSpy = createMarkerSpy(DEFAULT_MARKER_OPTIONS);
     createMarkerConstructorSpy(markerSpy).and.callThrough();
 
     const fixture = TestBed.createComponent(TestApp);
-    const fakeMap = {} as unknown as google.maps.Map;
     const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
     fixture.detectChanges();
 
     markerSpy.getAnimation.and.returnValue(null);
@@ -199,9 +171,6 @@ describe('MapMarker', () => {
     createMarkerConstructorSpy(markerSpy).and.callThrough();
 
     const fixture = TestBed.createComponent(TestApp);
-    const fakeMap = {} as unknown as google.maps.Map;
-    const markerComponent = fixture.debugElement.query(By.directive(MapMarker)).componentInstance;
-    markerComponent._setMap(fakeMap);
     fixture.detectChanges();
 
     expect(markerSpy.addListener)
@@ -234,13 +203,16 @@ describe('MapMarker', () => {
 
 @Component({
   selector: 'test-app',
-  template: `<map-marker [title]="title"
-                         [position]="position"
-                         [label]="label"
-                         [clickable]="clickable"
-                         [options]="options"
-                         (mapClick)="handleClick()"
-                         (positionChanged)="handlePositionChanged()"></map-marker>`,
+  template: `<google-map>
+               <map-marker [title]="title"
+                           [position]="position"
+                           [label]="label"
+                           [clickable]="clickable"
+                           [options]="options"
+                           (mapClick)="handleClick()"
+                           (positionChanged)="handlePositionChanged()">
+               </map-marker>
+             </google-map>`,
 })
 class TestApp {
   title?: string;

src/google-maps/map-marker/map-marker.ts

@@ -16,9 +16,11 @@ import {
   Output,
   ViewEncapsulation
 } from '@angular/core';
-import {BehaviorSubject, combineLatest, Observable, ReplaySubject, Subject} from 'rxjs';
+import {BehaviorSubject, combineLatest, Observable, Subject} from 'rxjs';
 import {map, takeUntil} from 'rxjs/operators';
 
+import {GoogleMap} from '../google-map/google-map';
+
 /**
  * Default options for the Google Maps marker component. Displays a marker
  * at the Googleplex.
@@ -198,30 +200,26 @@ export class MapMarker implements OnInit, OnDestroy {
       new BehaviorSubject<string|google.maps.MarkerLabel|undefined>(undefined);
   private readonly _clickable = new BehaviorSubject<boolean|undefined>(undefined);
 
-  private readonly _map = new ReplaySubject<google.maps.Map>(1);
-
   private readonly _destroy = new Subject<void>();
 
   private readonly _listeners: google.maps.MapsEventListener[] = [];
 
-  private _hasMap = false;
-
   _marker?: google.maps.Marker;
 
+  constructor(private readonly googleMap: GoogleMap) {}
+
   ngOnInit() {
     const combinedOptionsChanges = this._combineOptions();
 
-    combineLatest(this._map, combinedOptionsChanges)
-        .pipe(takeUntil(this._destroy))
-        .subscribe(([googleMap, options]) => {
-          if (this._marker) {
-            this._marker.setOptions(options);
-          } else {
-            this._marker = new google.maps.Marker(options);
-            this._marker.setMap(googleMap);
-            this._initializeEventHandlers();
-          }
-        });
+    combinedOptionsChanges.pipe(takeUntil(this._destroy)).subscribe(options => {
+      if (this._marker) {
+        this._marker.setOptions(options);
+      } else {
+        this._marker = new google.maps.Marker(options);
+        this._marker.setMap(this.googleMap._googleMap);
+        this._initializeEventHandlers();
+      }
+    });
   }
 
   ngOnDestroy() {
@@ -235,13 +233,6 @@ export class MapMarker implements OnInit, OnDestroy {
     }
   }
 
-  _setMap(googleMap: google.maps.Map) {
-    if (!this._hasMap) {
-      this._map.next(googleMap);
-      this._hasMap = true;
-    }
-  }
-
   /**
    * See
    * developers.google.com/maps/documentation/javascript/reference/marker#Marker.getAnimation
@@ -339,16 +330,15 @@ export class MapMarker implements OnInit, OnDestroy {
   }
 
   private _combineOptions(): Observable<google.maps.MarkerOptions> {
-    return combineLatest(
-               this._options, this._title, this._position, this._label, this._clickable, this._map)
-        .pipe(map(([options, title, position, label, clickable, googleMap]) => {
+    return combineLatest(this._options, this._title, this._position, this._label, this._clickable)
+        .pipe(map(([options, title, position, label, clickable]) => {
           const combinedOptions: google.maps.MarkerOptions = {
             ...options,
             title: title || options.title,
             position: position || options.position,
             label: label || options.label,
             clickable: clickable !== undefined ? clickable : options.clickable,
-            map: googleMap || null,
+            map: this.googleMap._googleMap || null,
           };
           return combinedOptions;
         }));

src/google-maps/testing/fake-google-map-utils.ts

@@ -68,11 +68,15 @@ export function createMarkerConstructorSpy(markerSpy: jasmine.SpyObj<google.maps
         return markerSpy;
       });
   const testingWindow: TestingWindow = window;
-  testingWindow.google = {
-    maps: {
-      'Marker': markerConstructorSpy,
-    },
-  };
+  if (testingWindow.google && testingWindow.google.maps) {
+    testingWindow.google.maps['Marker'] = markerConstructorSpy;
+  } else {
+    testingWindow.google = {
+      maps: {
+        'Marker': markerConstructorSpy,
+      },
+    };
+  }
   return markerConstructorSpy;
 }