GeezerVison is showing github.com — Angular Element is not removed and readded to the DOM properly · Issue #38778 · angular/angular
Angular Element is not removed and readded to the DOM properly #38778Description🐞 bug report
Affected Package
The issue is caused by package @angular/elements
Is this a regression?
No
Description
When an Angular Element is detached from the DOM and then re-attached, none of the event bindings work (see minimal reproduction for demo). I am a Google engineer and this is causing issues for us (internal bug filed here: http://b/168062117).
I talked to @kseamon about this and he thinks the issue is located in either of these 2 places:
|
disconnectedCallback(): void { |
|
disconnect() { |
|
this.runInZone(() => { |
|
// Return if there is no componentRef or the component is already scheduled for destruction |
|
if (this.componentRef === null || this.scheduledDestroyFn !== null) { |
|
return; |
|
} |
|
|
|
// Schedule the component to be destroyed after a small timeout in case it is being |
|
// moved elsewhere in the DOM |
|
this.scheduledDestroyFn = scheduler.schedule(() => { |
|
if (this.componentRef !== null) { |
|
this.componentRef.destroy(); |
|
this.componentRef = null; |
|
} |
|
}, DESTROY_DELAY); |
|
}); |
|
} |
Specifically, this.scheduledDestroyFn in not set to null in the scheduled destroy and so may be called again upon connect. I'm testing adding one line to see if it fixes the immediate issue.
However, even with this fix, there will still exist a tiny race condition that occurs when an element is scheduled to be destroyed and then connects before the destroy executes.
🔬 Minimal Reproduction
https://stackblitz.com/edit/angular-ovchwr?file=src%2Findex.html
🔥 Exception or Error
No user-visible exception
🌍 Your Environment
Angular Version:
I'm running it against the latest Angular in google3. Reacted by 0N1rick, Massimo Artizzu, Pranesh, HK, Patryk Szafran and Zoltán Lehoczky Reacted by Pawel Lbs Activitygkalpak commented on Sep 10, 2020 You are right, @yefim / @kseamon. It seems that by not setting this.scheduledDestroyFn to null when the component is destroyed, we hit this branch the next time the element is inserted into the DOM (and exit early, without instantiating the component).
I am not clear what you meant when you said "there will still exist a tiny race condition". Can you elaborate? gkalpak commented on Sep 10, 2020 To make sure we are on the same page, here is what the intention is with the current implementation:
- When the custom element is removed from the DOM, the underlying component is scheduled for destruction after a short timeout (currently 10ms).
- If the custom element is inserted back into the DOM before the timeout expires, then the component destruction is canceled and we keep the same component instance.
- If the custom element is inserted back into the DOM after the timeout, then the component has been destroyed and we should create a new component instance.
This last bullet point is what we are failing to do at the moment, as mentioned in the previous comment (which is a bug), but that was the intention.
The reason we destroy the component instance is that there is some Angular-specific cleaning up that needs to happen when the element (and thus the underlying component instance) is no longer necessary (i.e. removed from the DOM for good).
The reason we have the short delay between removing the custom element from the DOM and destroying the component instance is to account for cases where you want to "move" the element around (i.e. remove it from its current location in the DOM and immediately insert it at a new location). In that case, it would be a waste to destroy the component synchronously, only to re-instantiate it right after.
From a quick look, it seems that fixing the current bug (i.e. allowing the component to be re-instantiated when the host element is re-inserted into the DOM) is not trivial. For example, how do we handle content projection. The original nodes have been extracted from the host element the first time, so they are no longer there for us to extract. (We could cache them and re-use the same nodes, but I wouldn't be surprized if this broke in some edge cases.)
Even if we managed to fix this and allow the component to be re-instantiated, the internal state of the original component instance would not have been retained. This would lead to unexpected UI differences.
The main problem here is that there is no good way to automatically infer whether a removed element is intended to be re-used some time in the future (and thus the underlying component instance should be retained) or whether it is not needed any more (and thus the underlying component instance should be destroyed).
Therefore, we have to resort to unreliable heuristics ("if you haven't added it back after 10ms you probably don't need it").
An alternative approach would be to provide a way for people to denote that they intend to re-use a particular custom element and therefore the underlying component instance should not be destroyed after the element has been removed from the DOM.
Something like:
myCustomElement.remove();
// I might reuse this in the future. Do not destroy the underlying component.
myCustomElement.ngRetainComponentInstance(); As a work-around, you can basically do this manually today (by using some private properties 😇):
myCustomElement.remove();
myCustomElement.ngElementStrategy.scheduledDestroyFn()
myCustomElement.ngElementStrategy.scheduledDestroyFn = null; Reacted by 0N1rick and Markos da Silva kseamon commented on Sep 10, 2020 [Deleted comment as I was incorrectly thinking that there could be multiple elements assigned to a single strategy instance]
Disregard the part about the race condition - I think I was wrong about that. The comments below stand. Reacted by George Kalpakas kseamon commented on Sep 10, 2020 For the content projection issue, could we just put the projected elements back into the element when we disconnect? kseamon commented on Sep 10, 2020 There's another issue as well - when the first instance of the component is destroyed, Angular's animation engine then tries to remove the custom element node. So after the new one is created, we're seeing the animation engine circle back and remove the custom element which destroys the component again.
This was probably never observed before as you wont get this far unless you first null out the scheduledDestroyFn first. kseamon commented on Sep 10, 2020 Proposed solution for this one: create a div within the custom element and attach our component to it. This might be a slightly breaking change but I think will fix this issue. yefim commented on Sep 11, 2020 After setting scheduledDestroyFn to null when the component is destroyed and implementing the div fix in the comment above, I also noticed that Angular will not initialize the inputs with the values from the DOM. Neither preExistingValues nor initialInputValues were set. This led me to finding a related issue #30848 about HTML attributes. I modified the connectedCallback hook to always populate preExistingValues and to check the DOM attributes (via this.getAttribute). This seems to work as expected.
Here's the diff to get that last bit working:
The code could definitely be cleaned up and there may be edge cases I'm missing but it achieves my goal of initializing a new component with the correct inputs set. I noticed that for the first ever connect, the attributeChangedCallback hook is called and that is what sets the inputs. I'm open to any feedback :) gkalpak commented on Sep 13, 2020 HazzMan2409 commented on Sep 7, 2023 Bumped into this one today. Regarding the the code posted for the workaround I found that calling the scheduled destroy function actually cancels the destroy function. Doing this, means that ngOnDestroy is still called:
myCustomElement.remove();
myCustomElement.ngElementStrategy.scheduledDestroyFn = null; RazeiXello commented on Sep 19, 2023 #38778 (comment)
The work-around at the bottom of #38778 (comment) does not work for us for 2 reasons: (1) the component is destroyed by the @angular/animations package (I've filed #38962 for that) and (2) the inputs are not reinitialized from the HTML attributes when it reconnects.
I also bumped into this recently, we found that using a setter seems to give consistent results when trying to assign values from inputs on any subsequent initializations of an angular/elements web component.
private _myValue = '';
@Input()
set myValue(value: string) {
if (value) {
this._myValue = value;
}
}The caveat is that the setters seem to run after the child component loaded in the router-outlet tries to access the value, which was solved with a boolean that's set to true in the afterViewInit function.
Something like this in the AppComponent:
<div *ngIf="isInit">
<router-outlet></router-outlet>
</div> public isInit = false;
// allows the setters to execute
ngAfterViewInit(): void {
this.isInit = true;
}Doesn't solve the root issue, but has unblocked our work thus far. rahulwakade commented on Aug 26, 2024 Can we have an update on this from Angular team? straeger commented on Sep 6, 2024 Hello,
I also faced this issue within a component of our project. It was hard to reproduce. We have the effect that the components where directly destroyed when they where already created and rendered in the DOM.
After the initial rendering and navigating away within the outer angular app via ng-routing and re route to the component which uses our customElements we had the effect that from time to time ( 1 - 8 times ) the component directly was destroyed and not rendered.
I also had to wrap the constructor and overwrite the disconnectedCallback (like @aetzlecx did on Sep 14, 2021) to do nothing to get a stable rendering.
To me it looks like their is still an issue with @angular/elements 17.3.12 and attaching and detaching components to the DOM.
Unfortunately I currently have no time to create a stackblitz to reproduce this issue. Maybe I find some time later. jberndsen-planon commented on Jul 18, 2025 I can confirm we face these precise challenges still in Angular 19. Reacted by aetzlecx, GT Team and Igor Augustyński added 2 commits that reference this issue on Aug 8, 2025This issue will close once commit a405a02 is merged into the 'main' branch.a405a02 This issue will close once commit c8dc8b1 is merged into the 'main' branch.c8dc8b1 added 5 commits that reference this issue on Sep 26, 2025This issue will close once commit 8bc2d7e is merged into the 'main' branch.8bc2d7e This issue will close once commit 80029a8 is merged into the 'main' branch.80029a8 This issue will close once commit 90b516c is merged into the 'main' branch.90b516c This issue will close once commit 580d1d2 is merged into the 'main' branch.580d1d2 This issue will close once commit 58f52f6 is merged into the 'main' branch.58f52f6
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|
