-
Notifications
You must be signed in to change notification settings - Fork 27.4k
fix(*): do not break when modules are used with angular-loader
#14794
Conversation
This could be a problem when using `angular-loader` and the `angular-sanitize` file happens to get loaded before the core `angular` file.
This could be a problem when using `angular-loader` and the `angular-route` file happens to get loaded before the core `angular` file.
This could be a problem when using `angular-loader` and the `angular-messages` file happens to get loaded before the core `angular` file.
…vailable This could be a problem when using `angular-loader` and the `angular-message-format` file happens to get loaded before the core `angular` file.
This could be a problem when using `angular-loader` and the `angular-animate` file happens to get loaded before the core `angular` file.
@@ -309,7 +308,6 @@ describe('HTML', function() { | |||
|
|||
describe('htmlSanitizerWriter', function() { | |||
/* global htmlSanitizeWriter: false */ | |||
if (angular.isUndefined(window.htmlSanitizeWriter)) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
We should have tests to ensure this doesn't break later. |
@Narretz I agree. Are we now testing all the modules in a test run where the core angular source files are not loaded? |
When we test modules, we incude the bundled What @Narretz probably means, is that we should have a test were we use I'm not sure how we could force the module files to be loaded before the core |
Could we set-up an e2e test that uses the loader? |
I suppose we could (although we would probably need some extra dependencies, such as a script loader - angular-seed is using https://github.com/ded/script.js). The thing is, if the module files aren't loaded (as in downloaded from the server) before the core Maybe we can use the script loader to guarantee the order. |
BTW, this would fix #9140. |
What if we create an e2e test that simply loads the files in the "wrong" order: <script src="angular-loader.js"></script>
<script src="angular-resource.js"></script>
<script src="angular-animate.js"></script>
<script src="angular.js"></script> I think that the browser ensures that these files will run in the order in which they appear in the DOM, right? |
Obviously 😃 |
(Previous commits fixed angular#9140.) Closes angular#9140
I added a commit. The test does indeed break on our current master. |
Great! LGTM - please merge |
…ular-loader` Some modules used to assume that the angular helpers would always be available when their script was executed. This could be a problem when using `angular-loader` and the module file happened to get loaded before the core `angular.js` file. This commit fixes the issue by delaying the access to angular helpers, until they are guaranteed to be available. Affected modules: - `ngAnimate` - `ngMessageFormat` - `ngMessages` - `ngRoute` - `ngSanitize` Fixes #9140 Closes #14794
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix.
What is the current behavior? (You can also link to an open issue here)
When using
angular-loader
, you should be able to load the other scripts in any order. Yet, some of the external modules were assuming thatwindow.angular
would be available as soon as the script was loaded and executed. If the external module file happened to get loaded before the core angular file, things would fall apart.What is the new behavior (if this is a feature change)?
When using
angular-loader
, you are be able to load the other scripts in any order.Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Docs have been added / updated (for bug fixes / features)Other information: