Skip to content

Commit

Permalink
fix(google-maps): handle trying to access the map before it has been …
Browse files Browse the repository at this point in the history
…initialized

As things are set up at the moment, the Google `Map` object will be initialized once the API has loaded, however all of the methods on the `GoogleMap` component assume that the object will always be defined. This means that if any of the methods are called before it is initialized, we'll throw a null pointer error. These changes switch the type to `| undefined` and add guards around all the usages.
  • Loading branch information
crisbeto committed Nov 24, 2019
1 parent 49527e5 commit 29afab4
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 30 deletions.
72 changes: 45 additions & 27 deletions src/google-maps/google-map/google-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
@Output() zoomChanged = new EventEmitter<void>();

private _mapEl: HTMLElement;
_googleMap!: UpdatedGoogleMap;
_googleMap: UpdatedGoogleMap | undefined;

/** Whether we're currently rendering inside a browser. */
private _isBrowser: boolean;
Expand Down Expand Up @@ -244,8 +244,7 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
this._googleMapChanges = this._initializeMap(combinedOptionsChanges);
this._googleMapChanges.subscribe((googleMap: google.maps.Map) => {
this._googleMap = googleMap as UpdatedGoogleMap;

this._initializeEventHandlers();
this._initializeEventHandlers(this._googleMap);
});

this._watchForOptionsChanges();
Expand All @@ -267,23 +266,29 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
fitBounds(
bounds: google.maps.LatLngBounds|google.maps.LatLngBoundsLiteral,
padding?: number|google.maps.Padding) {
this._googleMap.fitBounds(bounds, padding);
if (this._googleMap) {
this._googleMap.fitBounds(bounds, padding);
}
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.panBy
*/
panBy(x: number, y: number) {
this._googleMap.panBy(x, y);
if (this._googleMap) {
this._googleMap.panBy(x, y);
}
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.panTo
*/
panTo(latLng: google.maps.LatLng|google.maps.LatLngLiteral) {
this._googleMap.panTo(latLng);
if (this._googleMap) {
this._googleMap.panTo(latLng);
}
}

/**
Expand All @@ -293,111 +298,125 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
panToBounds(
latLngBounds: google.maps.LatLngBounds|google.maps.LatLngBoundsLiteral,
padding?: number|google.maps.Padding) {
this._googleMap.panToBounds(latLngBounds, padding);
if (this._googleMap) {
this._googleMap.panToBounds(latLngBounds, padding);
}
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getBounds
*/
getBounds(): google.maps.LatLngBounds|null {
return this._googleMap.getBounds() || null;
const googleMap = this._googleMap;
return (googleMap ? googleMap.getBounds() : null) || null;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getCenter
* @deprecated Type to be changed to `google.maps.LatLng|null`.
* @breaking-change 10.0.0
*/
getCenter(): google.maps.LatLng {
return this._googleMap.getCenter();
return this._googleMap ? this._googleMap.getCenter() : null!;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getClickableIcons
*/
getClickableIcons(): boolean {
return this._googleMap.getClickableIcons();
return this._googleMap ? this._googleMap.getClickableIcons() : false;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getHeading
*/
getHeading(): number {
return this._googleMap.getHeading();
return this._googleMap ? this._googleMap.getHeading() : 0;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getMapTypeId
*/
getMapTypeId(): google.maps.MapTypeId|string {
return this._googleMap.getMapTypeId();
return this._googleMap ? this._googleMap.getMapTypeId() : '';
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getProjection
*/
getProjection(): google.maps.Projection|null {
return this._googleMap.getProjection();
return this._googleMap ? this._googleMap.getProjection() : null;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getStreetView
*
* @deprecated Return type to be changed to `google.maps.StreetViewPanorama|null`.
* @breaking-change 10.0.0
*/
getStreetView(): google.maps.StreetViewPanorama {
return this._googleMap.getStreetView();
return this._googleMap!.getStreetView();
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getTilt
*/
getTilt(): number {
return this._googleMap.getTilt();
return this._googleMap ? this._googleMap.getTilt() : 0;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.getZoom
*/
getZoom(): number {
return this._googleMap.getZoom();
return this._googleMap ? this._googleMap.getZoom() : 0;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.controls
*/
get controls(): Array<google.maps.MVCArray<Node>> {
return this._googleMap.controls;
return this._googleMap ? this._googleMap.controls : [];
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.data
* @deprecated Type to be changed to `google.maps.StreetViewPanorama|null`.
* @breaking-change 10.0.0
*/
get data(): google.maps.Data {
return this._googleMap.data;
return this._googleMap ? this._googleMap.data : null!;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.mapTypes
* @deprecated Return type to be changed to `google.maps.MapTypeRegistry|null`.
* @breaking-change 10.0.0
*/
get mapTypes(): google.maps.MapTypeRegistry {
return this._googleMap.mapTypes;
return this._googleMap ? this._googleMap.mapTypes : null!;
}

/**
* See
* https://developers.google.com/maps/documentation/javascript/reference/map#Map.overlayMapTypes
* @deprecated Return type to be changed to `google.maps.MVCArray<google.maps.MapType>|null`.
* @breaking-change 10.0.0
*/
get overlayMapTypes(): google.maps.MVCArray<google.maps.MapType> {
return this._googleMap.overlayMapTypes;
return this._googleMap ? this._googleMap.overlayMapTypes : null!;
}

private _setSize() {
Expand All @@ -423,9 +442,8 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
private _initializeMap(optionsChanges: Observable<google.maps.MapOptions>):
Observable<google.maps.Map> {
return optionsChanges.pipe(
take(1), map(options => {
return new google.maps.Map(this._mapEl, options);
}),
take(1),
map(options => new google.maps.Map(this._mapEl, options)),
shareReplay(1));
}

Expand Down Expand Up @@ -457,7 +475,7 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
});
}

private _initializeEventHandlers() {
private _initializeEventHandlers(googleMap: UpdatedGoogleMap) {
// Ensure that we don't leak if called multiple times.
this._clearListeners();

Expand All @@ -484,7 +502,7 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
]);
eventHandlers.forEach((eventHandler: EventEmitter<void>, name: string) => {
if (eventHandler.observers.length > 0) {
this._listeners.push(this._googleMap.addListener(name, () => {
this._listeners.push(googleMap.addListener(name, () => {
eventHandler.emit();
}));
}
Expand All @@ -493,13 +511,13 @@ export class GoogleMap implements OnChanges, OnInit, OnDestroy {
(eventHandler: EventEmitter<google.maps.MouseEvent>, name: string) => {
if (eventHandler.observers.length > 0) {
this._listeners.push(
this._googleMap.addListener(name, (event: google.maps.MouseEvent) => {
googleMap.addListener(name, (event: google.maps.MouseEvent) => {
eventHandler.emit(event);
}));
}
});
if (this.mapClick.observers.length > 0) {
this._listeners.push(this._googleMap.addListener(
this._listeners.push(googleMap.addListener(
'click', (event: google.maps.MouseEvent|google.maps.IconMouseEvent) => {
this.mapClick.emit(event);
}));
Expand Down
4 changes: 2 additions & 2 deletions src/google-maps/map-marker/map-marker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ export class MapMarker implements OnInit, OnDestroy {

combinedOptionsChanges.pipe(take(1)).subscribe(options => {
this._marker = new google.maps.Marker(options);
this._marker.setMap(this._googleMap._googleMap);
this._marker.setMap(this._googleMap._googleMap || null);
this._initializeEventHandlers();
});

Expand Down Expand Up @@ -341,7 +341,7 @@ export class MapMarker implements OnInit, OnDestroy {
position: position || options.position,
label: label || options.label,
clickable: clickable !== undefined ? clickable : options.clickable,
map: this._googleMap._googleMap || null,
map: this._googleMap._googleMap || undefined,
};
return combinedOptions;
}));
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/google-maps/google-maps.d.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export declare class GoogleMap implements OnChanges, OnInit, OnDestroy {
_googleMap: UpdatedGoogleMap;
_googleMap: UpdatedGoogleMap | undefined;
boundsChanged: EventEmitter<void>;
center: google.maps.LatLngLiteral | google.maps.LatLng;
centerChanged: EventEmitter<void>;
Expand Down

0 comments on commit 29afab4

Please sign in to comment.