Unmount angular by using the proper rootScope #24
Conversation
It looks as though the way we were getting the rootScope is incorrect.
The following is true:
```js
angular.injector(['ng']).get('$rootScope') !== angular.injector(['ng']).get('$rootScope')
```
In other words, the rootScope is not the same each time it is queried with this method. Whereas this is true:
```js
angular.element(getContainerEl().childNodes[0]).scope() === angular.element(getContainerEl().childNodes[0]).scope()
```
The problem was noticed when calling `angular.injector(['ng']).get('$rootScope').$destroy()` no children components `.$on('$destroy')` events were getting fired. More importantly, the `$onInit` of components wasn't getting fired on remount. When switching to the new way of querying the `$rootScope` and calling `$destroy()` all events properly fire.
| @@ -68,7 +68,9 @@ function mount() { | |||
|
|
|||
| function unmount() { | |||
| return new Promise((resolve, reject) => { | |||
| let rootScope = angular.injector(['ng']).get('$rootScope'); | |||
| let rootElement = angular.element(getContainerEl().childNodes[0]); | |||
There was a problem hiding this comment.
This is querying the first element that angular mounted which should have the $rootScope.
There was a problem hiding this comment.
Only concern is for this to throw an uncaught error
There was a problem hiding this comment.
Are we sure that the first childNode is the correct one?
There was a problem hiding this comment.
Could we do something like getContainerEl().querySelector('[ng-scope]')
There was a problem hiding this comment.
And if so, does angular put that ng-scope attribute on both in debug mode and production mode??
There was a problem hiding this comment.
@joeldenning There is no ng-scope attribute :( Maybe because I am debugging in prod!
There was a problem hiding this comment.
@kentmclean if it does throw an error, what should the outcome be? My thoughts are if there is an error, it is a catastrophic error. Is it something we could ignore? Maybe if we mount the app and unmount it super fast (before the dom has been built) then the [0] could be undefined?
There was a problem hiding this comment.
@joeldenning there is a className present with ng-scope
There was a problem hiding this comment.
@blittle - an error here would be unexpected for sure, probably due to a timing issue or an unintended re-unloading of an app. For our customers I'd want the error eaten as gracefully as possible. Then for our own purposes (and I assume this is already happening) we want to have the error alerted on through Sentry with relevant autoTrace stack.
| @@ -68,7 +72,9 @@ function mount() { | |||
|
|
|||
| function unmount() { | |||
| return new Promise((resolve, reject) => { | |||
| let rootScope = angular.injector(['ng']).get('$rootScope'); | |||
| let rootElement = angular.element(getContainerEl().querySelector(`#${SINGLE_SPA_ELEMENT_ID}`)); | |||
There was a problem hiding this comment.
Not using getElementById because it isn't available at a node level and I don't want to use document.getElementById because there might be more than one instance of this element.
|
Lgtm nice work finding this! |
|
🚶 |
|
👍 |
It looks as though the way we were getting the rootScope is incorrect.
The following is true:
In other words, the rootScope is not the same each time it is queried with this method. Whereas this is true:
The problem was noticed when calling
angular.injector(['ng']).get('$rootScope').$destroy()no children components.$on('$destroy')events were getting fired. More importantly, the$onInitof components wasn't getting fired on remount. When switching to the new way of querying the$rootScopeand calling$destroy()all events properly fire.