Skip to content

Commit

Permalink
refactor(compiler): Completely eliminate BindingFlags.
Browse files Browse the repository at this point in the history
It turns out that `BindingFlags.BindingTargetsTemplate` is actally a redundant property! It will be true in either of the following cases:
1. The template is a normal non-structural `ng-template`. We already know this from `TemplateKind`.
2. The binding came from `templateAttrs` (instead of `attrs`). We have this information in `BindingFlags.IsStructuralTemplateAttribute`.

Therefore, I can just eliminate `BindingFlags.BindingTargetsTemplate`. There's no reason to keep `BindingFlags` around for a single value, so I convert `BindingFlags.IsStructuralTemplateAttribute` to a boolean parameter (with the eventual goal of eliminating it entirely).
  • Loading branch information
dylhunn committed Dec 9, 2023
1 parent a8aef4c commit 9d0fa36
Showing 1 changed file with 11 additions and 33 deletions.
44 changes: 11 additions & 33 deletions packages/compiler/src/template/pipeline/src/ingest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -772,20 +772,6 @@ const BINDING_KINDS = new Map<e.BindingType, ir.BindingKind>([
[e.BindingType.Animation, ir.BindingKind.Animation],
]);

enum TemplateBindingFlags {
None = 0b000,

/**
* The binding belongs to the `<ng-template>` side of a `t.Template`.
*/
BindingTargetsTemplate = 0b01,

/**
* The binding is on a structural directive.
*/
IsStructuralTemplateAttribute = 0b10,
}

/**
* Checks whether the given template is a plain ng-template (as opposed to another kind of template
* such as a structural directive template or control flow template). This is checked based on the
Expand Down Expand Up @@ -875,40 +861,32 @@ function ingestElementBindings(
function ingestTemplateBindings(
unit: ViewCompilationUnit, op: ir.ElementOpBase, template: t.Template,
templateKind: ir.TemplateKind|null): void {
let flags: TemplateBindingFlags = TemplateBindingFlags.None;
let bindings = new Array<ir.BindingOp|ir.ExtractedAttributeOp|null>();

if (isPlainTemplate(template)) {
flags |= TemplateBindingFlags.BindingTargetsTemplate;
}

const templateAttrFlags = flags | TemplateBindingFlags.BindingTargetsTemplate |
TemplateBindingFlags.IsStructuralTemplateAttribute;
for (const attr of template.templateAttrs) {
if (attr instanceof t.TextAttribute) {
bindings.push(createTemplateBinding(
unit, op.xref, e.BindingType.Attribute, attr.name, o.literal(attr.value), null,
SecurityContext.NONE, true, templateAttrFlags, templateKind, asMessage(attr.i18n),
attr.sourceSpan));
SecurityContext.NONE, true, true, templateKind, asMessage(attr.i18n), attr.sourceSpan));
} else {
bindings.push(createTemplateBinding(
unit, op.xref, attr.type, attr.name, astOf(attr.value), attr.unit, attr.securityContext,
false, templateAttrFlags, templateKind, asMessage(attr.i18n), attr.sourceSpan));
false, true, templateKind, asMessage(attr.i18n), attr.sourceSpan));
}
}

for (const attr of template.attributes) {
// Attribute literal bindings, such as `attr.foo="bar"`.
bindings.push(createTemplateBinding(
unit, op.xref, e.BindingType.Attribute, attr.name, o.literal(attr.value), null,
SecurityContext.NONE, true, flags, templateKind, asMessage(attr.i18n), attr.sourceSpan));
SecurityContext.NONE, true, false, templateKind, asMessage(attr.i18n), attr.sourceSpan));
}

for (const input of template.inputs) {
// All dynamic bindings (both attribute and property bindings).
bindings.push(createTemplateBinding(
unit, op.xref, input.type, input.name, astOf(input.value), input.unit,
input.securityContext, false, flags, templateKind, asMessage(input.i18n),
input.securityContext, false, false, templateKind, asMessage(input.i18n),
input.sourceSpan));
}

Expand All @@ -933,7 +911,6 @@ function ingestTemplateBindings(
output.sourceSpan));
}


// TODO: Perhaps we could do this in a phase? (It likely wouldn't change the slot indices.)
if (bindings.some(b => b?.i18nMessage) !== null) {
unit.create.push(
Expand All @@ -944,10 +921,12 @@ function ingestTemplateBindings(
function createTemplateBinding(
view: ViewCompilationUnit, xref: ir.XrefId, type: e.BindingType, name: string,
value: e.AST|o.Expression, unit: string|null, securityContext: SecurityContext,
isTextBinding: boolean, flags: TemplateBindingFlags, templateKind: ir.TemplateKind|null,
i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): ir.BindingOp|
ir.ExtractedAttributeOp|null {
if (!(flags & TemplateBindingFlags.BindingTargetsTemplate) &&
isTextBinding: boolean, isStructuralTemplateAttribute: boolean,
templateKind: ir.TemplateKind|null, i18nMessage: i18n.Message|null,
sourceSpan: ParseSourceSpan): ir.BindingOp|ir.ExtractedAttributeOp|null {
const bindingTargetsTemplate =
isStructuralTemplateAttribute || templateKind === ir.TemplateKind.NgTemplate;
if (!bindingTargetsTemplate &&
(type === e.BindingType.Property || type === e.BindingType.Class ||
type === e.BindingType.Style)) {
// Because this binding doesn't really target the ng-template, it must be a binding on an inner
Expand All @@ -967,8 +946,7 @@ function createTemplateBinding(
return ir.createBindingOp(
xref, BINDING_KINDS.get(type)!, name,
convertAstWithInterpolation(view.job, value, i18nMessage), unit, securityContext,
isTextBinding, !!(flags & TemplateBindingFlags.IsStructuralTemplateAttribute), templateKind,
i18nMessage ?? null, sourceSpan);
isTextBinding, isStructuralTemplateAttribute, templateKind, i18nMessage ?? null, sourceSpan);
}

function makeListenerHandlerOps(
Expand Down

0 comments on commit 9d0fa36

Please sign in to comment.