Skip to content

Commit fbaa196

Browse files
IgorMinarIgorMinar
committed
chore($browser): remove the addJs method
this was never meant to be a public api used by apps. I refactored the code to hide the functionality. BREAKING CHANGE: $browser.addJs method was removed apps that depended on this functionality should either use many of the existing script loaders or create a simple helper method specific to the app.
1 parent 13d5528 commit fbaa196

6 files changed

Lines changed: 80 additions & 99 deletions

File tree

‎src/ng/browser.js‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -343,39 +343,6 @@ function Browser(window, document, body, $log, $sniffer) {
343343
// Misc API
344344
//////////////////////////////////////////////////////////////
345345

346-
347-
/**
348-
* @ngdoc method
349-
* @name angular.module.ng.$browser#addJs
350-
* @methodOf angular.module.ng.$browser
351-
*
352-
* @param {string} url Url to js file
353-
*
354-
* @description
355-
* Adds a script tag to the head.
356-
*/
357-
self.addJs = function(url, done) {
358-
// we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
359-
// - fetches local scripts via XHR and evals them
360-
// - adds and immediately removes script elements from the document
361-
var script = rawDocument.createElement('script');
362-
363-
script.type = 'text/javascript';
364-
script.src = url;
365-
366-
if (msie) {
367-
script.onreadystatechange = function() {
368-
/loaded|complete/.test(script.readyState) && done && done();
369-
};
370-
} else {
371-
if (done) script.onload = script.onerror = done;
372-
}
373-
374-
body[0].appendChild(script);
375-
376-
return script;
377-
};
378-
379346
/**
380347
* Returns current <base href>
381348
* (always relative - without domain)

‎src/ng/httpBackend.js‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,11 +26,11 @@ var XHR = window.XMLHttpRequest || function() {
2626
function $HttpBackendProvider() {
2727
this.$get = ['$browser', '$window', '$document', function($browser, $window, $document) {
2828
return createHttpBackend($browser, XHR, $browser.defer, $window.angular.callbacks,
29-
$document[0].body, $window.location.protocol.replace(':', ''));
29+
$document[0], $window.location.protocol.replace(':', ''));
3030
}];
3131
}
3232

33-
function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locationProtocol) {
33+
function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, locationProtocol) {
3434
// TODO(vojta): fix the signature
3535
return function(method, url, post, callback, headers, timeout, withCredentials) {
3636
$browser.$$incOutstandingRequestCount();
@@ -42,15 +42,14 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locati
4242
callbacks[callbackId].data = data;
4343
};
4444

45-
var script = $browser.addJs(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
45+
jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
4646
function() {
4747
if (callbacks[callbackId].data) {
4848
completeRequest(callback, 200, callbacks[callbackId].data);
4949
} else {
5050
completeRequest(callback, -2);
5151
}
5252
delete callbacks[callbackId];
53-
body.removeChild(script);
5453
});
5554
} else {
5655
var xhr = new XHR();
@@ -100,4 +99,28 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locati
10099
$browser.$$completeOutstandingRequest(noop);
101100
}
102101
};
102+
103+
function jsonpReq(url, done) {
104+
// we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
105+
// - fetches local scripts via XHR and evals them
106+
// - adds and immediately removes script elements from the document
107+
var script = rawDocument.createElement('script'),
108+
doneWrapper = function() {
109+
rawDocument.body.removeChild(script);
110+
if (done) done();
111+
}
112+
113+
script.type = 'text/javascript';
114+
script.src = url;
115+
116+
if (msie) {
117+
script.onreadystatechange = function() {
118+
if (/loaded|complete/.test(script.readyState)) doneWrapper();
119+
};
120+
} else {
121+
script.onload = script.onerror = doneWrapper;
122+
}
123+
124+
rawDocument.body.appendChild(script);
125+
};
103126
}

‎src/ngMock/angular-mocks.js‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,6 @@ angular.mock.$Browser = function() {
137137
self.baseHref = function() {
138138
return this.$$baseHref;
139139
};
140-
141-
self.$$scripts = [];
142-
self.addJs = function(url, done) {
143-
var script = {url: url, done: done};
144-
self.$$scripts.push(script);
145-
return script;
146-
};
147140
};
148141
angular.mock.$Browser.prototype = {
149142

‎test/ng/browserSpecs.js‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -526,19 +526,6 @@ describe('browser', function() {
526526
});
527527
});
528528

529-
describe('addJs', function() {
530-
it('should append a script tag to body', function() {
531-
browser.addJs('http://localhost/bar.js');
532-
expect(scripts.length).toBe(1);
533-
expect(scripts[0].src).toBe('http://localhost/bar.js');
534-
expect(scripts[0].id).toBe('');
535-
});
536-
537-
it('should return the appended script element', function() {
538-
var script = browser.addJs('http://localhost/bar.js');
539-
expect(script).toBe(scripts[0]);
540-
});
541-
});
542529

543530
describe('baseHref', function() {
544531
var jqDocHead;

‎test/ng/httpBackendSpec.js‎

Lines changed: 53 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
describe('$httpBackend', function() {
22

33
var $backend, $browser, callbacks,
4-
xhr, fakeBody, callback;
4+
xhr, fakeDocument, callback;
55

66
// TODO(vojta): should be replaced by $defer mock
77
function fakeTimeout(fn, delay) {
@@ -21,8 +21,24 @@ describe('$httpBackend', function() {
2121
beforeEach(inject(function($injector) {
2222
callbacks = {counter: 0};
2323
$browser = $injector.get('$browser');
24-
fakeBody = {removeChild: jasmine.createSpy('body.removeChild')};
25-
$backend = createHttpBackend($browser, MockXhr, fakeTimeout, callbacks, fakeBody);
24+
fakeDocument = {
25+
$$scripts: [],
26+
createElement: jasmine.createSpy('createElement').andCallFake(function() {
27+
return {};
28+
}),
29+
body: {
30+
appendChild: jasmine.createSpy('body.appendChid').andCallFake(function(script) {
31+
fakeDocument.$$scripts.push(script);
32+
}),
33+
removeChild: jasmine.createSpy('body.removeChild').andCallFake(function(script) {
34+
var index = indexOf(fakeDocument.$$scripts, script);
35+
if (index != -1) {
36+
fakeDocument.$$scripts.splice(index, 1);
37+
}
38+
})
39+
}
40+
};
41+
$backend = createHttpBackend($browser, MockXhr, fakeTimeout, callbacks, fakeDocument);
2642
callback = jasmine.createSpy('done');
2743
}));
2844

@@ -131,32 +147,44 @@ describe('$httpBackend', function() {
131147
});
132148

133149
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
134-
expect($browser.$$scripts.length).toBe(1);
150+
expect(fakeDocument.$$scripts.length).toBe(1);
135151

136-
var script = $browser.$$scripts.shift(),
137-
url = script.url.match(SCRIPT_URL);
152+
var script = fakeDocument.$$scripts.shift(),
153+
url = script.src.match(SCRIPT_URL);
138154

139155
expect(url[1]).toBe('http://example.org/path');
140156
callbacks[url[2]]('some-data');
141-
script.done();
157+
158+
if (script.onreadystatechange) {
159+
script.readyState = 'complete';
160+
script.onreadystatechange();
161+
} else {
162+
script.onload()
163+
}
142164

143165
expect(callback).toHaveBeenCalledOnce();
144166
});
145167

146168

147169
it('should clean up the callback and remove the script', function() {
148170
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
149-
expect($browser.$$scripts.length).toBe(1);
171+
expect(fakeDocument.$$scripts.length).toBe(1);
172+
150173

151-
var script = $browser.$$scripts.shift(),
152-
callbackId = script.url.match(SCRIPT_URL)[2];
174+
var script = fakeDocument.$$scripts.shift(),
175+
callbackId = script.src.match(SCRIPT_URL)[2];
153176

154177
callbacks[callbackId]('some-data');
155-
script.done();
178+
179+
if (script.onreadystatechange) {
180+
script.readyState = 'complete';
181+
script.onreadystatechange();
182+
} else {
183+
script.onload()
184+
}
156185

157186
expect(callbacks[callbackId]).toBeUndefined();
158-
expect(fakeBody.removeChild).toHaveBeenCalledOnce();
159-
expect(fakeBody.removeChild).toHaveBeenCalledWith(script);
187+
expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script);
160188
});
161189

162190

@@ -167,21 +195,26 @@ describe('$httpBackend', function() {
167195
});
168196

169197
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
170-
expect($browser.$$scripts.length).toBe(1);
171-
172-
$browser.$$scripts.shift().done();
198+
expect(fakeDocument.$$scripts.length).toBe(1);
199+
200+
var script = fakeDocument.$$scripts.shift();
201+
if (script.onreadystatechange) {
202+
script.readyState = 'complete';
203+
script.onreadystatechange();
204+
} else {
205+
script.onload()
206+
}
173207
expect(callback).toHaveBeenCalledOnce();
174208
});
175209

176210

177211
it('should set url to current location if not specified or empty string', function() {
178212
$backend('JSONP', undefined, null, callback);
179-
expect($browser.$$scripts[0].url).toBe($browser.url());
180-
$browser.$$scripts.shift();
213+
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
214+
fakeDocument.$$scripts.shift();
181215

182216
$backend('JSONP', '', null, callback);
183-
expect($browser.$$scripts[0].url).toBe($browser.url());
184-
$browser.$$scripts.shift();
217+
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
185218
});
186219

187220

‎test/ngMock/angular-mocksSpec.js‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,28 +3,6 @@
33
describe('ngMock', function() {
44
var noop = angular.noop;
55

6-
describe('$browser', function() {
7-
8-
describe('addJs', function() {
9-
10-
it('should store url, done', inject(function($browser) {
11-
var url = 'some.js',
12-
done = angular.noop;
13-
14-
$browser.addJs(url, done);
15-
16-
var script = $browser.$$scripts.shift();
17-
expect(script.url).toBe(url);
18-
expect(script.done).toBe(done);
19-
}));
20-
21-
22-
it('should return the script object', inject(function($browser) {
23-
expect($browser.addJs('some.js', null, noop)).toBe($browser.$$scripts[0]);
24-
}));
25-
});
26-
});
27-
286

297
describe('TzDate', function() {
308

0 commit comments

Comments
 (0)