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

Conditionals and content projection #54840

Closed
pkozlowski-opensource opened this issue Mar 13, 2024 · 4 comments
Closed

Conditionals and content projection #54840

pkozlowski-opensource opened this issue Mar 13, 2024 · 4 comments
Assignees
Labels
area: core Issues related to the framework runtime core: content projection core: control flow Issues related to the built-in control flow (@if, @for, @switch) state: has PR
Milestone

Comments

@pkozlowski-opensource
Copy link
Member

pkozlowski-opensource commented Mar 13, 2024

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

There is a surprising interaction between our static (creation time) content projection system and the new control flow (although this is not specific to the new control flow - the same can be observed with ngIf).

To illustrate the surprising scenario let's consider a component with content projection with a selector (<ng-content select="some-content">) and content to project wrapped in the control flow @if statement with the else branch:

<with-content>
      @if(false) {          
        <some-content />
      } @else {
        I'm not the content queried for, don't project me! 
      }
</with-content>

https://stackblitz.com/edit/stackblitz-starters-57dynz?file=src%2Fmain.ts

As surprising as it may be, the I'm not the content queried for, don't project me! text node will be taken as content to project (!). This is consistent with the existing implementation which will project the whole @if container and container capturing the some-content as a tag name of a root node.

While we can't fix all issues with the content projection, we could remedy this one by

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/stackblitz-starters-57dynz?file=src%2Fmain.ts

Please provide the exception or error you saw

No response

Please provide the environment you discovered this bug in (run ng version)

No response

Anything else?

No response

@pkozlowski-opensource pkozlowski-opensource added area: core Issues related to the framework runtime core: control flow Issues related to the built-in control flow (@if, @for, @switch) labels Mar 13, 2024
@ngbot ngbot bot added this to the needsTriage milestone Mar 13, 2024
@csisy-bt4w
Copy link

csisy-bt4w commented Mar 14, 2024

I'd like to add to this, I'm not sure however, if related or not. Some selectors do not work if the content itself is generated (e.g. with @if or @for). Consider the following example:

@Component({
  selector: 'app-container',
  standalone: true,
  template: `
    <div>Contains:</div>
    <ng-content select="[routerLink]"/>
  `
})
export class AppContainerComponent {}

By using this component, an element with a routerLink directive is only projected if it is given as a content directly:

<app-container>
  <button [routerLink]="'/1'">/1</button>
</app-container>

However, if generated dynamically, then nothing is projected, nothing is shown:

<app-container>
  @if (true) {
    <button [routerLink]="'/2'">/2</button>
  } @else {
    <button [routerLink]="'/3'">/3</button>
  }
</app-container>
<app-container>
  @for (link of links; track link) {
    <button [routerLink]="link">{{ link }}</button>
  }
</app-container>

Please note: if I use a component attribute name as a selector (e.g. [app-custom]), then it works. I think this is because in this case, the html element itself contains an attribute with the same name.

Edit:
Also, this is a regression, we updated our project to the latest (17.3.0) version, and it was working in version 17.1.0

Edit 2:
This seems to be a bug related to the new template pipeline. In the tsconfig if we added "useTemplatePipeline": false then everything works as expected.

@pkozlowski-opensource
Copy link
Member Author

@csisy-btw could you please open a separate issue for your use-case, ideally with a minimal reproduction scenario in a stackblitz? Otherwise we might miss this one. Thnx!

pkozlowski-opensource added a commit to pkozlowski-opensource/angular that referenced this issue Mar 15, 2024
This commit changes the way we use containers to insert conditional content.
Previously if and switch conditional would always use the first content as the
insertion container. This strategy interfered with content projection that
projects entire containers - as the consequence content for _all_ cases would
end up in slot matched by the first container. This could be very surprising
as desicribed in angular#54840

After the change each conditional content is projected into its own container.
This means that content projection can match more than one container and
result in correct display.

Fixes angular#54840
pkozlowski-opensource added a commit to pkozlowski-opensource/angular that referenced this issue Mar 15, 2024
This commit changes the way we use containers to insert conditional content.
Previously if and switch conditional would always use the first content as the
insertion container. This strategy interfered with content projection that
projects entire containers - as the consequence content for _all_ cases would
end up in slot matched by the first container. This could be very surprising
as desicribed in angular#54840

After the change each conditional content is projected into its own container.
This means that content projection can match more than one container and
result in correct display.

Fixes angular#54840
@pkozlowski-opensource
Copy link
Member Author

PR: #54879

crisbeto pushed a commit to crisbeto/angular that referenced this issue Mar 18, 2024
This commit changes the way we use containers to insert conditional content.
Previously if and switch conditional would always use the first content as the
insertion container. This strategy interfered with content projection that
projects entire containers - as the consequence content for _all_ cases would
end up in slot matched by the first container. This could be very surprising
as desicribed in angular#54840

After the change each conditional content is projected into its own container.
This means that content projection can match more than one container and
result in correct display.

Fixes angular#54840
ilirbeqirii pushed a commit to ilirbeqirii/angular that referenced this issue Apr 6, 2024
…ngular#54921)

This commit changes the way we use containers to insert conditional content.
Previously if and switch conditional would always use the first content as the
insertion container. This strategy interfered with content projection that
projects entire containers - as the consequence content for _all_ cases would
end up in slot matched by the first container. This could be very surprising
as desicribed in angular#54840

After the change each conditional content is projected into its own container.
This means that content projection can match more than one container and
result in correct display.

Fixes angular#54840

PR Close angular#54921
@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 Apr 22, 2024
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 core: content projection core: control flow Issues related to the built-in control flow (@if, @for, @switch) state: has PR
Projects
None yet
3 participants