Skip to content

Commit

Permalink
fix(LazyMapsAPILoader): multiple google maps api scripts on page
Browse files Browse the repository at this point in the history 
This prevents that we have multiple create script tags for the
google maps api.

Fixes #315
Fixes #775
Fixes #1260
  • Loading branch information
@sebholstein
sebholstein authored Mar 18, 2018
1 parent 2563cae commit 07de5a4
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 21 deletions.
52 changes: 35 additions & 17 deletions packages/core/services/maps-api-loader/lazy-maps-api-loader.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,46 +8,66 @@ import {MapsAPILoader} from './maps-api-loader';
describe('Service: LazyMapsAPILoader', () => {
let documentRef: DocumentRef;
let doc: any;
let windowRef: any;
let windowRef: WindowRef;
let windowObj: any;

beforeEach(() => {
doc = jasmine.createSpyObj<DocumentRef>('Document', ['createElement']);
doc = jasmine.createSpyObj<DocumentRef>('Document', ['createElement', 'getElementById']);
(<jasmine.Spy>doc.getElementById).and.returnValue(null);
doc.body = jasmine.createSpyObj('body', ['appendChild']);
documentRef = jasmine.createSpyObj<DocumentRef>('Document', ['getNativeDocument']);
(<any>documentRef.getNativeDocument).and.returnValue(doc);
windowRef = {};
});
(<jasmine.Spy>documentRef.getNativeDocument).and.returnValue(doc);

windowRef = jasmine.createSpyObj<WindowRef>('windowRef', ['getNativeWindow']);
windowObj = {};
(<jasmine.Spy>windowRef.getNativeWindow).and.returnValue(windowObj);

it('should create the default script URL', () => {
TestBed.configureTestingModule({
providers: [
{provide: MapsAPILoader, useClass: LazyMapsAPILoader},
{provide: WindowRef, useValue: windowRef}, {provide: DocumentRef, useValue: documentRef}
{provide: WindowRef, useValue: windowRef},
{provide: DocumentRef, useValue: documentRef}
]
});
});

inject([MapsAPILoader], (loader: LazyMapsAPILoader) => {
it('should create the default script URL', inject([MapsAPILoader], (loader: LazyMapsAPILoader) => {
interface Script {
src?: string;
async?: boolean;
defer?: boolean;
type?: string;
id?: string;
}
const scriptElem: Script = {};
(<jasmine.Spy>doc.createElement).and.returnValue(scriptElem);
doc.body = jasmine.createSpyObj('body', ['appendChild']);

loader.load();
expect(doc.createElement).toHaveBeenCalled();
expect(doc.createElement).toHaveBeenCalledWith('script');
expect(scriptElem.type).toEqual('text/javascript');
expect(scriptElem.async).toEqual(true);
expect(scriptElem.defer).toEqual(true);
expect(scriptElem.src).toBeDefined();
expect(scriptElem.id).toEqual('agmGoogleMapsApiScript');
expect(scriptElem.src).toContain('https://maps.googleapis.com/maps/api/js');
expect(scriptElem.src).toContain('v=3');
expect(scriptElem.src).toContain('callback=angular2GoogleMapsLazyMapsAPILoader');
expect(scriptElem.src).toContain('callback=agmLazyMapsAPILoader');
expect(doc.body.appendChild).toHaveBeenCalledWith(scriptElem);
});
});
}));

it('should not append a second script to body when theres already one with the fixed ID', inject([MapsAPILoader], (loader: LazyMapsAPILoader) => {
(<jasmine.Spy>doc.getElementById).and.returnValue(document.createElement('script'));
loader.load();
expect(doc.body.appendChild).not.toHaveBeenCalledWith();
}));

it('should not append a second script to body when window.google.maps is defined', inject([MapsAPILoader], (loader: LazyMapsAPILoader) => {
windowObj.google = {
maps: {}
};
loader.load();
expect(doc.body.appendChild).not.toHaveBeenCalledWith();
}));

it('should load the script via http when provided', () => {
const lazyLoadingConf:
Expand All @@ -56,7 +76,8 @@ describe('Service: LazyMapsAPILoader', () => {
TestBed.configureTestingModule({
providers: [
{provide: MapsAPILoader, useClass: LazyMapsAPILoader},
{provide: WindowRef, useValue: windowRef}, {provide: DocumentRef, useValue: documentRef},
{provide: WindowRef, useValue: windowRef},
{provide: DocumentRef, useValue: documentRef},
{provide: LAZY_MAPS_API_CONFIG, useValue: lazyLoadingConf}
]
});
Expand All @@ -70,13 +91,10 @@ describe('Service: LazyMapsAPILoader', () => {
}
const scriptElem: Script = {};
(<jasmine.Spy>doc.createElement).and.returnValue(scriptElem);
doc.body = jasmine.createSpyObj('body', ['appendChild']);

loader.load();
expect(doc.createElement).toHaveBeenCalled();
expect(scriptElem.src).toContain('http://maps.googleapis.com/maps/api/js');
expect(scriptElem.src).toContain('v=3');
expect(scriptElem.src).toContain('callback=angular2GoogleMapsLazyMapsAPILoader');
expect(doc.body.appendChild).toHaveBeenCalledWith(scriptElem);
});
});
Expand Down
15 changes: 11 additions & 4 deletions packages/core/services/maps-api-loader/lazy-maps-api-loader.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Inject, Injectable, InjectionToken} from '@angular/core';
import {Inject, Injectable, InjectionToken, Optional} from '@angular/core';

import {DocumentRef, WindowRef} from '../../utils/browser-globals';

Expand All @@ -14,7 +14,7 @@ export enum GoogleMapsScriptProtocol {
* Token for the config of the LazyMapsAPILoader. Please provide an object of type {@link
* LazyMapsAPILoaderConfig}.
*/
export const LAZY_MAPS_API_CONFIG = new InjectionToken('angular-google-maps LAZY_MAPS_API_CONFIG');
export const LAZY_MAPS_API_CONFIG = new InjectionToken<LazyMapsAPILoaderConfigLiteral>('angular-google-maps LAZY_MAPS_API_CONFIG');

/**
* Configuration for the {@link LazyMapsAPILoader}.
Expand Down Expand Up @@ -84,8 +84,9 @@ export class LazyMapsAPILoader extends MapsAPILoader {
protected _config: LazyMapsAPILoaderConfigLiteral;
protected _windowRef: WindowRef;
protected _documentRef: DocumentRef;
protected readonly _SCRIPT_ID: string = 'agmGoogleMapsApiScript';

constructor(@Inject(LAZY_MAPS_API_CONFIG) config: any, w: WindowRef, d: DocumentRef) {
constructor(@Optional() @Inject(LAZY_MAPS_API_CONFIG) config: any = null, w: WindowRef, d: DocumentRef) {
super();
this._config = config || {};
this._windowRef = w;
Expand All @@ -99,6 +100,11 @@ export class LazyMapsAPILoader extends MapsAPILoader {
return Promise.resolve();
}

if (this._documentRef.getNativeDocument().getElementById(this._SCRIPT_ID)) {
// this can happen in HMR situations or Stackblitz.io editors.
return Promise.resolve();
}

if (this._scriptLoadingPromise) {
return this._scriptLoadingPromise;
}
Expand All @@ -107,7 +113,8 @@ export class LazyMapsAPILoader extends MapsAPILoader {
script.type = 'text/javascript';
script.async = true;
script.defer = true;
const callbackName: string = `angular2GoogleMapsLazyMapsAPILoader`;
script.id = this._SCRIPT_ID;
const callbackName: string = `agmLazyMapsAPILoader`;
script.src = this._getScriptSrc(callbackName);

this._scriptLoadingPromise = new Promise<void>((resolve: Function, reject: Function) => {
Expand Down

1 comment on commit 07de5a4

@leocaseiro
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that fix @SebastianM, would you mind bumping a new version?
beta.3 perhaps?

Thanks

Please sign in to comment.