Replaced mutation observer with root element zone changes #184
Conversation
| @@ -117,5 +117,5 @@ export abstract class BaseAdapter { | |||
|
|
|||
| abstract serializeComponent(el: any, event: string): TreeNode; | |||
|
|
|||
| abstract cleanup(): void; | |||
| // abstract cleanup(): void; | |||
There was a problem hiding this comment.
let's stay away from keeping commented out code, we have history with source control and can always go back if needed. I would remove it.
|
@sumitarora overall LGTM minus the comments. Otherwise I am not sure if I followed what is happening here 100% as I am a but out of the loop with regards to what API they provided us with regards to being notified of changes to the components. I am giving it a 👍 , but let's have @vanessayuenn or @bertrandk give a second look through. |
|
The suspicion is that we're leaking a lot of DOM nodes in this routine. @sumitarora, did you plan on addressing that in this PR or treating it separately? In any case, aside from the small cleanup advocated, this looks good to me. |
|
@bertrandk I will do another PR for DOM issues |
| @@ -109,11 +133,6 @@ export class Angular2Adapter extends BaseAdapter { | |||
| }; | |||
| } | |||
|
|
|||
| cleanup(): void { | |||
There was a problem hiding this comment.
the new listener doesn't need to be disconnected/unsubscribed?
|
only had one question about the unsubscribe, but it looks good otherwise. 👍 |
Replaced mutation observer with root element zone changes
Should fix: #181 and #164