Removing "previous" Overlay containers breaks other OverlayContainer instances

[AlexBachmann]

A fix from Jul 18, 2019 removes all "previous" overlay container elements of the DOM:
036729d#diff-80f4f593448bba8d614136a94c0c9c5c
@crisbeto @jelbourn

The problem here is, that there might be other OverlayContainer instances active in the application (as an example: Child components might create a new FullScreenOverlayContainer instances, if their injector does not contain an active instance yet) , and by selecting all DOM elements by a class selector, you are detaching these overlay container elements from the DOM as well.

const containerClass = 'cdk-overlay-container';
const previousContainers = this._document.getElementsByClassName(containerClass);
// Remove any old containers. This can happen when transitioning from the server to the client.
for (let i = 0; i < previousContainers.length; i++) {
  previousContainers[i].parentNode!.removeChild(previousContainers[i]);
}

I hope I find the time to reproduce this issue in stackblitz in the near future, but the issue is quite easy to mentally comprehend:

Imagine two compoennts compA and compB, and each of them have separate OverlayContainer instances injected to them.

Now CompA attaches a portal to its OverlayContainer. No container element has been created for his OverlayContainer instance yet, so the _createContainer() method is being called. All "previous" containers are being cleared (which isn't a problem yet) and a new one is created. The overlay is being displayed correctly.

Now CompB attaches a portal to its component overlay. No Container Element has been created for his OverlayContainer instance yet, so the _createContainer() method is being called. All "previous" containers are being cleared, including the container element of the OverlayContainer of compA. The overlay of comB is being display correctly.

Now CompA tries to attach another portal to its OverlayContainer. His OverlayContainer still references the container element it has created before even though this element has been removed from the DOM by this point.

if (!this._containerElement) { this._createContainer(); }

So, NO new container element is being created.. However, this container element has been detached from the DOM earlier by the OverlayContainer from compB. So now we can attach whatever we want to this container, they won't be displayed in the DOM.

To fix this issue, three solutions should be examined:

1. If it's true that there always should only be ONE container element, independent of the OverlayContainer instance, then the getContainerElement() method should not check, whether or not a reference exists in the particular instance, but instead perform a dom query, as it's done currently done, when existing container elements are removed
2. If it's true that there should only be ONE container element per OverlayContainer instance, than these containers should have some sort of ID attached to them, so we only remove the containers refering to this instance
3. The proposed fix targets a situation caused by server-side-rendering using angular universal. Maybe there is a different solution to address this issue.

Reproduction

I hope I can build a stackblitz for this issue soon. However, I think I've explained the problem

Expected Behavior

What behavior were you expecting to see?

Actual Behavior

What behavior did you actually see?

Environment

• Angular: 8.2
• CDK/Material: 8.1
• Browser(s):
• Operating System (e.g. Windows, macOS, Ubuntu):

[flash-me]

Omg I'm having exactly the same issue right now. Even though I worked around by providing CustomOverlayContainer, I think this is indeed an issue, since one platform should not affect another platform...