From fbaa1968b7c596ccb63ea8b4be1d3bd92eda50d8 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Mon, 9 Apr 2012 14:27:14 -0700 Subject: [PATCH] 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. --- src/ng/browser.js | 33 --------------- src/ng/httpBackend.js | 31 ++++++++++++-- src/ngMock/angular-mocks.js | 7 --- test/ng/browserSpecs.js | 13 ------ test/ng/httpBackendSpec.js | 73 +++++++++++++++++++++++--------- test/ngMock/angular-mocksSpec.js | 22 ---------- 6 files changed, 80 insertions(+), 99 deletions(-) diff --git a/src/ng/browser.js b/src/ng/browser.js index 376c3fd4756b..465114c5b132 100644 --- a/src/ng/browser.js +++ b/src/ng/browser.js @@ -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 * (always relative - without domain) diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index b2c14b3f2ae6..d2784efa46b2 100644 --- a/src/ng/httpBackend.js +++ b/src/ng/httpBackend.js @@ -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) { +function createHttpBackend($browser, XHR, $browserDefer, callbacks, rawDocument, locationProtocol) { // TODO(vojta): fix the signature return function(method, url, post, callback, headers, timeout, withCredentials) { $browser.$$incOutstandingRequestCount(); @@ -42,7 +42,7 @@ 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); @@ -50,7 +50,6 @@ function createHttpBackend($browser, XHR, $browserDefer, callbacks, body, locati completeRequest(callback, -2); } delete callbacks[callbackId]; - body.removeChild(script); }); } else { var xhr = new XHR(); @@ -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); + }; } diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 8c50c90ea796..42d921e373fc 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -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 = { diff --git a/test/ng/browserSpecs.js b/test/ng/browserSpecs.js index 4563d14b4dc0..77894d4314af 100644 --- a/test/ng/browserSpecs.js +++ b/test/ng/browserSpecs.js @@ -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; diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index 91c0574dc445..06b63c3c8c8d 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -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) { @@ -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'); })); @@ -131,14 +147,20 @@ 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(); }); @@ -146,17 +168,23 @@ describe('$httpBackend', function() { 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); }); @@ -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()); }); diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index c656e940cf90..4b2666b73b09 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -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() {