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

Diagnostic to flag cases where control flow can break content projection #53190

Closed
wants to merge 4 commits into from

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Nov 27, 2023

Note: this is a resubmit of #52726.

These changes are a follow-up to the fix from #52414. They add a new diagnostic to the compiler that will flag cases where an element would've been projected into a specific slot of a component, but it isn't because it exists inside of a control flow node that has multiple root nodes. Aside from the diagnostic, I have to move some code around so it can be reused. Includes the following commits:

refactor(compiler): expose utility for creating CSS selectors from AST nodes

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in compiler-cli.

refactor(compiler-cli): expose ng-content selectors and preserveWhitespaces during template type checking

These changes expose the ngContentSelectors and preserveWhitespaces metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

test(core): add tests for control flow content projection with ng-container

The control flow projection diagnostic will mention ng-container as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

fix(compiler-cli): add diagnostic for control flow that prevents content projection

This is a follow-up to the fix from #52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.

…T nodes

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in `compiler-cli`.
…spaces during template type checking

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.
…tainer

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.
@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer area: compiler Issues related to `ngc`, Angular's template compiler target: minor This PR is targeted for the next minor release labels Nov 27, 2023
@ngbot ngbot bot added this to the Backlog milestone Nov 27, 2023
…ent projection

This is a follow-up to the fix from angular#52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.
@crisbeto crisbeto force-pushed the control-flow-diagnostic-again branch from 385331c to 2233493 Compare November 27, 2023 09:29
@crisbeto
Copy link
Member Author

Passing TGP

@crisbeto crisbeto marked this pull request as ready for review November 27, 2023 11:35
Copy link
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed-for: public-api

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 27, 2023
@pkozlowski-opensource
Copy link
Member

This PR was merged into the repository by commit 4c1d69e.

pkozlowski-opensource pushed a commit that referenced this pull request Nov 28, 2023
…spaces during template type checking (#53190)

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

PR Close #53190
pkozlowski-opensource pushed a commit that referenced this pull request Nov 28, 2023
…tainer (#53190)

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

PR Close #53190
pkozlowski-opensource pushed a commit that referenced this pull request Nov 28, 2023
…ent projection (#53190)

This is a follow-up to the fix from #52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.

PR Close #53190
Tweniee pushed a commit to Tweniee/angular that referenced this pull request Nov 29, 2023
…T nodes (angular#53190)

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in `compiler-cli`.

PR Close angular#53190
Tweniee pushed a commit to Tweniee/angular that referenced this pull request Nov 29, 2023
…spaces during template type checking (angular#53190)

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

PR Close angular#53190
Tweniee pushed a commit to Tweniee/angular that referenced this pull request Nov 29, 2023
…tainer (angular#53190)

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

PR Close angular#53190
crisbeto added a commit to crisbeto/angular that referenced this pull request Dec 1, 2023
…t projection diagnostic

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.
crisbeto added a commit to crisbeto/angular that referenced this pull request Dec 1, 2023
…t projection diagnostic

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.
crisbeto added a commit to crisbeto/angular that referenced this pull request Dec 1, 2023
…t projection diagnostic

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.
crisbeto added a commit to crisbeto/angular that referenced this pull request Dec 1, 2023
…t projection diagnostic

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.
dylhunn pushed a commit that referenced this pull request Dec 6, 2023
…t projection diagnostic (#53311)

These changes add an option to the `extendedDiagnostics` field that allows the check from #53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close #53311
crisbeto added a commit to crisbeto/angular that referenced this pull request Dec 6, 2023
…t projection diagnostic

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.
dylhunn pushed a commit that referenced this pull request Dec 6, 2023
…t projection diagnostic (#53387)

These changes add an option to the `extendedDiagnostics` field that allows the check from #53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close #53387
tbondwilkinson pushed a commit to tbondwilkinson/angular that referenced this pull request Dec 6, 2023
…T nodes (angular#53190)

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in `compiler-cli`.

PR Close angular#53190
tbondwilkinson pushed a commit to tbondwilkinson/angular that referenced this pull request Dec 6, 2023
…spaces during template type checking (angular#53190)

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

PR Close angular#53190
tbondwilkinson pushed a commit to tbondwilkinson/angular that referenced this pull request Dec 6, 2023
…tainer (angular#53190)

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

PR Close angular#53190
tbondwilkinson pushed a commit to tbondwilkinson/angular that referenced this pull request Dec 6, 2023
…ent projection (angular#53190)

This is a follow-up to the fix from angular#52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.

PR Close angular#53190
tbondwilkinson pushed a commit to tbondwilkinson/angular that referenced this pull request Dec 6, 2023
…t projection diagnostic (angular#53311)

These changes add an option to the `extendedDiagnostics` field that allows the check from angular#53190 to be disabled. This is a follow-up based on a recent discussion.

PR Close angular#53311
@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 29, 2023
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…T nodes (angular#53190)

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in `compiler-cli`.

PR Close angular#53190
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…spaces during template type checking (angular#53190)

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

PR Close angular#53190
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…tainer (angular#53190)

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

PR Close angular#53190
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this pull request Jan 23, 2024
…ent projection (angular#53190)

This is a follow-up to the fix from angular#52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.

PR Close angular#53190
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…T nodes (angular#53190)

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in `compiler-cli`.

PR Close angular#53190
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…spaces during template type checking (angular#53190)

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

PR Close angular#53190
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…tainer (angular#53190)

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

PR Close angular#53190
rlmestre pushed a commit to rlmestre/angular that referenced this pull request Jan 26, 2024
…ent projection (angular#53190)

This is a follow-up to the fix from angular#52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.

PR Close angular#53190
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…T nodes (angular#53190)

When doing directive matching in the compiler, we need to be able to create a selector from an AST node. We already have the utility, but these changes simplify the public API and expose it so it can be used in `compiler-cli`.

PR Close angular#53190
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…spaces during template type checking (angular#53190)

These changes expose the `ngContentSelectors` and `preserveWhitespaces` metadata to the TCB so they can be used in the next commit to implement a new diagnostic.

PR Close angular#53190
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…tainer (angular#53190)

The control flow projection diagnostic will mention `ng-container` as a workaround for projection multiple nodes. These changes add a couple of tests to ensure that the approach works.

PR Close angular#53190
amilamen pushed a commit to amilamen/angular that referenced this pull request Jan 26, 2024
…ent projection (angular#53190)

This is a follow-up to the fix from angular#52414. It adds a diagnostic that will tell users when a control flow is preventing its direct descendants from being projected into a specific component slot.

PR Close angular#53190
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants