@deferred silently does nothing if host also has structural directive

[leonsenft]

The following pattern silently drops the @deferred:

<my-comp *ngIf="showExample @deferred></my-comp>

A correct solution is to move the structural directive to a parent element:

<div *ngIf="showExample">
  <my-comp @deferred></my-comp>
</div>

Given this restriction, the compiler should warn or error if @deferred and a structural directive are placed on the same element.

[alorenzen]

We already have an error if there are two structural directives on the same element. I think it's reasonable to treat @deferred as a structural directive and error in this case as well.

[matanlurey]

Agree with @alorenzen, though it is confusing because it is an @ and not a *.

[matanlurey]

We agreed making this a static error is acceptable, and not strictly speaking a breaking change.

[matanlurey]

@leonsenft @alorenzen I guess that we'd have to do this in desugar_visitor.dart:

    if (astNode.annotations.isNotEmpty) {
      final i = astNode.annotations.indexWhere((ast) => ast.name == 'deferred');
      if (i != -1) {
        final deferredAst = astNode.annotations.removeAt(i);
        final origin = _toolFriendlyAstOrigin ? deferredAst : null;
        return EmbeddedTemplateAst.from(
          origin,
          childNodes: [astNode],
          hasDeferredComponent: true,
        );
      }
    }

... right?

[leonsenft]

Yeah, I think so.

[matanlurey]

It looks like this is less than straightforward, because:

1. We de-sugar *directives before parsing annotations.
2. The code that parses <template> doesn't even parse annotations.

Thoughts?

[leonsenft]

Why not check the annotations before de-sugaring *directives? Essentially swap these blocks: https://github.com/dart-lang/angular/blob/5ad73932b7885ce78b1371bbe79628f5fec3d6d6/angular_ast/lib/src/visitors/desugar_visitor.dart#L87-L102

[matanlurey]

That solves the * case but not the <template> one :)

[leonsenft]

Oh sorry I think I misunderstood your second point. You're concerned about

<template @deferred>
  ...
</template>

right?

<template> elements wouldn't have supported annotations at the time @deferred was added, but I think they now do to support @preserveWhitespace right?

Why not add the check for @deferred to visitEmbeddedTemplate() in desugar_visitor.dart?

[matanlurey]

I could, it just seemed messy. But reading this discussion, it probably is not. Thanks!

[matanlurey]

Fixed in 703fbda.