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

crisbeto · 2023-11-09T09:23:56Z

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.

crisbeto · 2023-11-09T09:32:59Z

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

+ * of a control flow expression *would have been projected* into a specific slot, if the control
+ * flow node didn't exist.
+ */
+class TcbControlFlowContentProjectionOp extends TcbOp {


I wasn't totally sure if another TcbOp was the right place to put this since it doesn't add any statements to the TCB, but I saw that we have some precedent for cases in the TcbDomSchemaCheckerOp. My main concern was to isolate the check from everything else in the TCB. The other option was to put it in a private method inside the Scope, but that class is pretty large as it is.

packages/compiler-cli/src/ngtsc/metadata/src/dts.ts

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

JoostK · 2023-11-09T19:38:19Z

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

+    const blockName = controlFlowNode instanceof TmplAstIfBlockBranch ? '@if' : '@for';
+    const lines = [
+      `Node matches the "${slotSelector}" slot of the "${
+          componentName}" component, but will not be projected because the surrounding ${


"will not be projected" may be inaccurate if there is a default slot. Perhaps this should have two forms ("will not be projected" and "will be projected into the default projection slot" or smth like that) or generalize to e.g. "will not be projected there".

I changed the wording a bit to mention that it won't be projected into that slot.

crisbeto · 2023-11-14T08:44:33Z

I've addressed most of the feedback and will bring up the discussion around using a warning versus an error later today.

jessicajaniuk

LGTM. I'm indifferent on warning vs error.

reviewed-for: fw-core, public-api

…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.

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

…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.

…nts content projection (#52726)" (#53012) This reverts commit b4d022e. PR Close #53012

… from AST nodes (#52726)" (#53012) This reverts commit adbea7b. PR Close #53012

…rveWhitespaces during template type checking (#52726)" (#53012) This reverts commit 4550a81. PR Close #53012

…h ng-container (#52726)" (#53012) This reverts commit 181f8e4. PR Close #53012

…nts content projection (#52726)" (#53012) This reverts commit b4d022e. PR Close #53012

…T nodes (angular#52726) 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#52726

…spaces during template type checking (angular#52726) 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#52726

…tainer (angular#52726) 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#52726

…ent projection (angular#52726) 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#52726

… from AST nodes (angular#52726)" (angular#53012) This reverts commit adbea7b. PR Close angular#53012

…rveWhitespaces during template type checking (angular#52726)" (angular#53012) This reverts commit 4550a81. PR Close angular#53012

…h ng-container (angular#52726)" (angular#53012) This reverts commit 181f8e4. PR Close angular#53012

…nts content projection (angular#52726)" (angular#53012) This reverts commit b4d022e. PR Close angular#53012

angular-automatic-lock-bot · 2023-12-18T00:09:13Z

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.}