Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Comments

Close side panel

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

Closed
crisbeto wants to merge 4 commits intoangular:mainangular/angular:mainfrom
crisbeto:control-flow-diagnostic-againcrisbeto/angular:control-flow-diagnostic-againCopy head branch name to clipboard
Closed

Diagnostic to flag cases where control flow can break content projection#53190
crisbeto wants to merge 4 commits intoangular:mainangular/angular:mainfrom
crisbeto:control-flow-diagnostic-againcrisbeto/angular:control-flow-diagnostic-againCopy head branch name to clipboard

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

@pullapprove pullapprove bot requested a review from jessicajaniuk November 27, 2023 15:59
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
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
@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.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.