Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
chore($browser): remove the addJs method
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@IgorMinar
IgorMinar committed Apr 10, 2012
1 parent 13d5528 commit fbaa196
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 99 deletions.
33 changes: 0 additions & 33 deletions src/ng/browser.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -343,39 +343,6 @@ function Browser(window, document, body, $log, $sniffer) {
// Misc API
//////////////////////////////////////////////////////////////


/**
* @ngdoc method
* @name angular.module.ng.$browser#addJs
* @methodOf angular.module.ng.$browser
*
* @param {string} url Url to js file
*
* @description
* Adds a script tag to the head.
*/
self.addJs = function(url, done) {
// we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
// - fetches local scripts via XHR and evals them
// - adds and immediately removes script elements from the document
var script = rawDocument.createElement('script');

script.type = 'text/javascript';
script.src = url;

if (msie) {
script.onreadystatechange = function() {
/loaded|complete/.test(script.readyState) && done && done();
};
} else {
if (done) script.onload = script.onerror = done;
}

body[0].appendChild(script);

return script;
};

/**
* Returns current <base href>
* (always relative - without domain)
Expand Down
31 changes: 27 additions & 4 deletions src/ng/httpBackend.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ var XHR = window.XMLHttpRequest || function() {
function $HttpBackendProvider() {
this.$get = ['$browser', '$window', '$document', function($browser, $window, $document) {
return createHttpBackend($browser, XHR, $browser.defer, $window.angular.callbacks,
$document[0].body, $window.location.protocol.replace(':', ''));
$document[0], $window.location.protocol.replace(':', ''));
}];
}

function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locationProtocol) {

This comment has been minimized.

Sign in to view

Copy link
@mbj

mbj Apr 4, 2013

Just curios, why this method takes 8 arguments? Why not pass in a decorated config object? This way features such as #1159 #1934 would be far easier to implement!
function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, locationProtocol) {
// TODO(vojta): fix the signature
return function(method, url, post, callback, headers, timeout, withCredentials) {
$browser.$$incOutstandingRequestCount();
Expand All @@ -42,15 +42,14 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locati
callbacks[callbackId].data = data;
};

var script = $browser.addJs(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
function() {
if (callbacks[callbackId].data) {
completeRequest(callback, 200, callbacks[callbackId].data);
} else {
completeRequest(callback, -2);
}
delete callbacks[callbackId];
body.removeChild(script);
});
} else {
var xhr = new XHR();
Expand Down Expand Up @@ -100,4 +99,28 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locati
$browser.$$completeOutstandingRequest(noop);
}
};

function jsonpReq(url, done) {
// we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
// - fetches local scripts via XHR and evals them
// - adds and immediately removes script elements from the document
var script = rawDocument.createElement('script'),
doneWrapper = function() {
rawDocument.body.removeChild(script);
if (done) done();
}

script.type = 'text/javascript';
script.src = url;

if (msie) {
script.onreadystatechange = function() {
if (/loaded|complete/.test(script.readyState)) doneWrapper();
};
} else {
script.onload = script.onerror = doneWrapper;
}

rawDocument.body.appendChild(script);
};
}
7 changes: 0 additions & 7 deletions src/ngMock/angular-mocks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,6 @@ angular.mock.$Browser = function() {
self.baseHref = function() {
return this.$$baseHref;
};

self.$$scripts = [];
self.addJs = function(url, done) {
var script = {url: url, done: done};
self.$$scripts.push(script);
return script;
};
};
angular.mock.$Browser.prototype = {

Expand Down
13 changes: 0 additions & 13 deletions test/ng/browserSpecs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,19 +526,6 @@ describe('browser', function() {
});
});

describe('addJs', function() {
it('should append a script tag to body', function() {
browser.addJs('http://localhost/bar.js');
expect(scripts.length).toBe(1);
expect(scripts[0].src).toBe('http://localhost/bar.js');
expect(scripts[0].id).toBe('');
});

it('should return the appended script element', function() {
var script = browser.addJs('http://localhost/bar.js');
expect(script).toBe(scripts[0]);
});
});

describe('baseHref', function() {
var jqDocHead;
Expand Down
73 changes: 53 additions & 20 deletions test/ng/httpBackendSpec.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
describe('$httpBackend', function() {

var $backend, $browser, callbacks,
xhr, fakeBody, callback;
xhr, fakeDocument, callback;

// TODO(vojta): should be replaced by $defer mock
function fakeTimeout(fn, delay) {
Expand All @@ -21,8 +21,24 @@ describe('$httpBackend', function() {
beforeEach(inject(function($injector) {
callbacks = {counter: 0};
$browser = $injector.get('$browser');
fakeBody = {removeChild: jasmine.createSpy('body.removeChild')};
$backend = createHttpBackend($browser, MockXhr, fakeTimeout, callbacks, fakeBody);
fakeDocument = {
$$scripts: [],
createElement: jasmine.createSpy('createElement').andCallFake(function() {
return {};
}),
body: {
appendChild: jasmine.createSpy('body.appendChid').andCallFake(function(script) {
fakeDocument.$$scripts.push(script);
}),
removeChild: jasmine.createSpy('body.removeChild').andCallFake(function(script) {
var index = indexOf(fakeDocument.$$scripts, script);
if (index != -1) {
fakeDocument.$$scripts.splice(index, 1);
}
})
}
};
$backend = createHttpBackend($browser, MockXhr, fakeTimeout, callbacks, fakeDocument);
callback = jasmine.createSpy('done');
}));

Expand Down Expand Up @@ -131,32 +147,44 @@ describe('$httpBackend', function() {
});

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

var script = $browser.$$scripts.shift(),
url = script.url.match(SCRIPT_URL);
var script = fakeDocument.$$scripts.shift(),
url = script.src.match(SCRIPT_URL);

expect(url[1]).toBe('http://example.org/path');
callbacks[url[2]]('some-data');
script.done();

if (script.onreadystatechange) {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload()
}

expect(callback).toHaveBeenCalledOnce();
});


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


var script = $browser.$$scripts.shift(),
callbackId = script.url.match(SCRIPT_URL)[2];
var script = fakeDocument.$$scripts.shift(),
callbackId = script.src.match(SCRIPT_URL)[2];

callbacks[callbackId]('some-data');
script.done();

if (script.onreadystatechange) {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload()
}

expect(callbacks[callbackId]).toBeUndefined();
expect(fakeBody.removeChild).toHaveBeenCalledOnce();
expect(fakeBody.removeChild).toHaveBeenCalledWith(script);
expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script);
});


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

$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
expect($browser.$$scripts.length).toBe(1);

$browser.$$scripts.shift().done();
expect(fakeDocument.$$scripts.length).toBe(1);

var script = fakeDocument.$$scripts.shift();
if (script.onreadystatechange) {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload()
}
expect(callback).toHaveBeenCalledOnce();
});


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

$backend('JSONP', '', null, callback);
expect($browser.$$scripts[0].url).toBe($browser.url());
$browser.$$scripts.shift();
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());
});


Expand Down
22 changes: 0 additions & 22 deletions test/ngMock/angular-mocksSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,6 @@
describe('ngMock', function() {
var noop = angular.noop;

describe('$browser', function() {

describe('addJs', function() {

it('should store url, done', inject(function($browser) {
var url = 'some.js',
done = angular.noop;

$browser.addJs(url, done);

var script = $browser.$$scripts.shift();
expect(script.url).toBe(url);
expect(script.done).toBe(done);
}));


it('should return the script object', inject(function($browser) {
expect($browser.addJs('some.js', null, noop)).toBe($browser.$$scripts[0]);
}));
});
});


describe('TzDate', function() {

Expand Down

0 comments on commit fbaa196

Please sign in to comment.