Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dynamic Component not insert into shadowDOM #39556

Closed
mhartington opened this issue Nov 4, 2020 · 5 comments
Closed

Dynamic Component not insert into shadowDOM #39556

mhartington opened this issue Nov 4, 2020 · 5 comments
Assignees
Labels
area: core Issues related to the framework runtime P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent shadow dom Issue related to Shadow DOM state: has PR
Milestone

Comments

@mhartington
Copy link
Contributor

🐞 bug report

Affected Package

The issue is caused by package @angular/10x

Is this a regression?

Unsure.

Description

When rendering a dynamic component via ComponentFactoryResolver and ViewContainerRef.createComponent, if the ViewContainerRef is a web component with ShadowDom, the component is never rendered into the DOM. This was brought to our attention through ionic-team/ionic-framework#22421

🔬 Minimal Reproduction

https://github.com/mhartington/ng-wc-component-factory-issue

🔥 Exception or Error

No exception or error is thrown

🌍 Your Environment

Angular Version:


Angular CLI: 10.2.0
Node: 14.11.0
OS: darwin x64

Angular: 10.2.1
... common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1002.0
@angular-devkit/build-angular   0.1002.0
@angular-devkit/core            10.0.8
@angular-devkit/schematics      10.0.8
@angular/cli                    10.2.0
@schematics/angular             10.0.8
@schematics/update              0.1002.0
rxjs                            6.5.5
typescript                      3.9.7

Anything else relevant?

It's worth noting that in the example, we have

<ion-content> <! -- web component -->
  <button (click)="add()">Add</button>
  <div #target></div>
</ion-content>

If the #target is changed to a ng-template or the ion-content is removed, the issue does not happen.

@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Nov 4, 2020
@ngbot ngbot bot added this to the needsTriage milestone Nov 4, 2020
@jelbourn jelbourn added the shadow dom Issue related to Shadow DOM label Nov 4, 2020
@jelbourn
Copy link
Member

jelbourn commented Nov 4, 2020

Just to clarify- you're expecting the dynamically created content to be rendered within the shadow root's light DOM?

@jelbourn jelbourn added the P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent label Nov 4, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Nov 4, 2020
@mhartington
Copy link
Contributor Author

Technically, yes. The internals of the component define a slot in the shadowRoot

<host>
  <shadow-root>
    <!-- default slot, render to light dom -->
    <slot></slot>
  </shadow-root>
</host>

So when the component is consumed

<comp-a>
  <div>Content gets composed in the default slot, but will render in light dom</div>
</comp-a>

The dom tree will be

<comp-a>
  #shadow-root
  <slot></slot>

<!-- light dom -->
  <div>Content gets composed in the default slot, but will render in light dom</div>
</comp-a>

So they dynamic component should just appear in the light DOM as it does with a regular div.
I hope I explained this well 😄

crisbeto added a commit to crisbeto/angular that referenced this issue Nov 7, 2020
… component

When a `ViewContainerRef` is injected, we dynamically create a comment node next to the host
so that it can be used as an anchor point for inserting views. The comment node is inserted
through the `appendChild` helper from `node_manipulation.ts` in most cases.

The problem with using `appendChild` here is that it has some extra logic which doesn't return
a parent `RNode` if an element is at the root of a component. I __think__ that this is a performance
optimization which is used to avoid inserting an element in one place in the DOM and then
moving it a bit later when it is projected. This can break down in some cases when creating
a `ViewContainerRef` for a non-component node at the root of another component like the following:

```
<root>
  <div #viewContainerRef></div>
</root>
```

In this case the `#viewContainerRef` node is at the root of a component so we intentionally don't
insert it, but since its anchor element was created manually, it'll never be projected. This will
prevent any views added through the `ViewContainerRef` from being inserted into the DOM.

These changes resolve the issue by not going through `appendChild` at all when creating a comment
node for `ViewContainerRef`. This should work identically since `appendChild` doesn't really do
anything with the T structures anyway, it only uses them to reach the relevant DOM nodes.

Fixes angular#39556.
crisbeto added a commit to crisbeto/angular that referenced this issue Nov 7, 2020
… component

When a `ViewContainerRef` is injected, we dynamically create a comment node next to the host
so that it can be used as an anchor point for inserting views. The comment node is inserted
through the `appendChild` helper from `node_manipulation.ts` in most cases.

The problem with using `appendChild` here is that it has some extra logic which doesn't return
a parent `RNode` if an element is at the root of a component. I __think__ that this is a performance
optimization which is used to avoid inserting an element in one place in the DOM and then
moving it a bit later when it is projected. This can break down in some cases when creating
a `ViewContainerRef` for a non-component node at the root of another component like the following:

```
<root>
  <div #viewContainerRef></div>
</root>
```

In this case the `#viewContainerRef` node is at the root of a component so we intentionally don't
insert it, but since its anchor element was created manually, it'll never be projected. This will
prevent any views added through the `ViewContainerRef` from being inserted into the DOM.

These changes resolve the issue by not going through `appendChild` at all when creating a comment
node for `ViewContainerRef`. This should work identically since `appendChild` doesn't really do
anything with the T structures anyway, it only uses them to reach the relevant DOM nodes.

Fixes angular#39556.
@crisbeto crisbeto self-assigned this Nov 7, 2020
@crisbeto
Copy link
Member

crisbeto commented Nov 7, 2020

#39599 should resolve it. For what it's worth, this has nothing to do with shadow DOM, but it breaks because you're injecting the ViewContainerRef from a node that's at the root of a component. Another way to work around it until the fix lands is to wrap the #testShadow element in a div.

@mhartington
Copy link
Contributor Author

Thanks for the information @crisbeto!

crisbeto added a commit to crisbeto/angular that referenced this issue Nov 10, 2020
… component

When a `ViewContainerRef` is injected, we dynamically create a comment node next to the host
so that it can be used as an anchor point for inserting views. The comment node is inserted
through the `appendChild` helper from `node_manipulation.ts` in most cases.

The problem with using `appendChild` here is that it has some extra logic which doesn't return
a parent `RNode` if an element is at the root of a component. I __think__ that this is a performance
optimization which is used to avoid inserting an element in one place in the DOM and then
moving it a bit later when it is projected. This can break down in some cases when creating
a `ViewContainerRef` for a non-component node at the root of another component like the following:

```
<root>
  <div #viewContainerRef></div>
</root>
```

In this case the `#viewContainerRef` node is at the root of a component so we intentionally don't
insert it, but since its anchor element was created manually, it'll never be projected. This will
prevent any views added through the `ViewContainerRef` from being inserted into the DOM.

These changes resolve the issue by not going through `appendChild` at all when creating a comment
node for `ViewContainerRef`. This should work identically since `appendChild` doesn't really do
anything with the T structures anyway, it only uses them to reach the relevant DOM nodes.

Fixes angular#39556.
atscott pushed a commit that referenced this issue Nov 12, 2020
… component (#39599)

When a `ViewContainerRef` is injected, we dynamically create a comment node next to the host
so that it can be used as an anchor point for inserting views. The comment node is inserted
through the `appendChild` helper from `node_manipulation.ts` in most cases.

The problem with using `appendChild` here is that it has some extra logic which doesn't return
a parent `RNode` if an element is at the root of a component. I __think__ that this is a performance
optimization which is used to avoid inserting an element in one place in the DOM and then
moving it a bit later when it is projected. This can break down in some cases when creating
a `ViewContainerRef` for a non-component node at the root of another component like the following:

```
<root>
  <div #viewContainerRef></div>
</root>
```

In this case the `#viewContainerRef` node is at the root of a component so we intentionally don't
insert it, but since its anchor element was created manually, it'll never be projected. This will
prevent any views added through the `ViewContainerRef` from being inserted into the DOM.

These changes resolve the issue by not going through `appendChild` at all when creating a comment
node for `ViewContainerRef`. This should work identically since `appendChild` doesn't really do
anything with the T structures anyway, it only uses them to reach the relevant DOM nodes.

Fixes #39556.

PR Close #39599
basherr pushed a commit to basherr/angular that referenced this issue Nov 14, 2020
… component (angular#39599)

When a `ViewContainerRef` is injected, we dynamically create a comment node next to the host
so that it can be used as an anchor point for inserting views. The comment node is inserted
through the `appendChild` helper from `node_manipulation.ts` in most cases.

The problem with using `appendChild` here is that it has some extra logic which doesn't return
a parent `RNode` if an element is at the root of a component. I __think__ that this is a performance
optimization which is used to avoid inserting an element in one place in the DOM and then
moving it a bit later when it is projected. This can break down in some cases when creating
a `ViewContainerRef` for a non-component node at the root of another component like the following:

```
<root>
  <div #viewContainerRef></div>
</root>
```

In this case the `#viewContainerRef` node is at the root of a component so we intentionally don't
insert it, but since its anchor element was created manually, it'll never be projected. This will
prevent any views added through the `ViewContainerRef` from being inserted into the DOM.

These changes resolve the issue by not going through `appendChild` at all when creating a comment
node for `ViewContainerRef`. This should work identically since `appendChild` doesn't really do
anything with the T structures anyway, it only uses them to reach the relevant DOM nodes.

Fixes angular#39556.

PR Close angular#39599
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent shadow dom Issue related to Shadow DOM state: has PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants