Skip to content

Commit

Permalink
todo message
Browse files Browse the repository at this point in the history
  • Loading branch information
@dylhunn
dylhunn committed Dec 6, 2023
1 parent 62e0f45 commit dc730d5
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 46 deletions.
10 changes: 8 additions & 2 deletions ...r-cli/test/compliance/test_cases/r3_view_compiler_i18n/element_attributes/TEST_CASES.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,16 @@
"extraChecks": [
"verifyPlaceholdersIntegrity",
"verifyUniqueConsts"
],
"files": [
{
"expected": "ng-template_interpolation_template.js",
"templatePipelineExpected": "ng-template_interpolation_template.pipeline.js",
"generated": "ng-template_interpolation.js"
}
]
}
],
"skipForTemplatePipeline": true
]
},
{
"description": "should support i18n attributes with interpolations on explicit <ng-template> elements with structural directives",
Expand Down
0 ...t_attributes/ng-template_interpolation.js → ...tes/ng-template_interpolation_template.js
View file
File renamed without changes.
18 changes: 18 additions & 0 deletions ...s/r3_view_compiler_i18n/element_attributes/ng-template_interpolation_template.pipeline.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
consts: () => {
__i18nMsg__('Hello {$interpolation}', [['interpolation', String.raw`\uFFFD0\uFFFD`]], {original_code: {'interpolation': '{{ name }}'}}, {})
return [
["title", $i18n_0$],
[__AttributeMarker.Bindings__, "title"]
];
},
template: function MyComponent_Template(rf, ctx) {
if (rf & 1) {
$r3$.ɵɵtemplate(0, MyComponent_ng_template_0_Template, 0, 0, "ng-template", 1);
$r3$.ɵɵi18nAttributes(1, 0);
}
if (rf & 2) {
$r3$.ɵɵi18nExp(ctx.name);
$r3$.ɵɵi18nApply(1);
}
}

66 changes: 40 additions & 26 deletions packages/compiler/src/template/pipeline/ir/src/ops/update.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {SecurityContext} from '../../../../../core';
import * as i18n from '../../../../../i18n/i18n_ast';
import * as o from '../../../../../output/output_ast';
import {ParseSourceSpan} from '../../../../../parse_util';
import {BindingKind, I18nExpressionFor, I18nParamResolutionTime, OpKind} from '../enums';
import {BindingKind, I18nExpressionFor, I18nParamResolutionTime, OpKind, TemplateKind} from '../enums';
import type {ConditionalCaseExpr} from '../expression';
import {SlotHandle} from '../handle';
import {Op, XrefId} from '../operations';
Expand Down Expand Up @@ -121,10 +121,12 @@ export interface BindingOp extends Op<UpdateOp> {
*/
isTextAttribute: boolean;

isStructuralTemplateAttribute: boolean;

/**
* Whether this binding is on a structural template.
*/
isStructuralTemplate: boolean;
templateKind: TemplateKind|null;

i18nContext: XrefId|null;
i18nMessage: i18n.Message|null;
Expand All @@ -138,8 +140,8 @@ export interface BindingOp extends Op<UpdateOp> {
export function createBindingOp(
target: XrefId, kind: BindingKind, name: string, expression: o.Expression|Interpolation,
unit: string|null, securityContext: SecurityContext, isTextAttribute: boolean,
isStructuralTemplate: boolean, i18nMessage: i18n.Message|null,
sourceSpan: ParseSourceSpan): BindingOp {
isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): BindingOp {
return {
kind: OpKind.Binding,
bindingKind: kind,
Expand All @@ -149,7 +151,8 @@ export function createBindingOp(
unit,
securityContext,
isTextAttribute,
isStructuralTemplate: isStructuralTemplate,
isStructuralTemplateAttribute,
templateKind,
i18nContext: null,
i18nMessage,
sourceSpan,
Expand Down Expand Up @@ -193,10 +196,13 @@ export interface PropertyOp extends Op<UpdateOp>, ConsumesVarsTrait, DependsOnSl
*/
sanitizer: o.Expression|null;

isStructuralTemplateAttribute: boolean;

/**
* Whether this binding is on a structural template.
* The kind of template targeted by the binding, or null if this binding does not target a
* template.
*/
isStructuralTemplate: boolean;
templateKind: TemplateKind|null;

i18nContext: XrefId|null;
i18nMessage: i18n.Message|null;
Expand All @@ -209,7 +215,8 @@ export interface PropertyOp extends Op<UpdateOp>, ConsumesVarsTrait, DependsOnSl
*/
export function createPropertyOp(
target: XrefId, name: string, expression: o.Expression|Interpolation,
isAnimationTrigger: boolean, securityContext: SecurityContext, isStructuralTemplate: boolean,
isAnimationTrigger: boolean, securityContext: SecurityContext,
isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
i18nContext: XrefId|null, i18nMessage: i18n.Message|null,
sourceSpan: ParseSourceSpan): PropertyOp {
return {
Expand All @@ -220,7 +227,8 @@ export function createPropertyOp(
isAnimationTrigger,
securityContext,
sanitizer: null,
isStructuralTemplate,
isStructuralTemplateAttribute,
templateKind,
i18nContext,
i18nMessage,
sourceSpan,
Expand Down Expand Up @@ -424,10 +432,13 @@ export interface AttributeOp extends Op<UpdateOp> {
*/
isTextAttribute: boolean;

isStructuralTemplateAttribute: boolean;

/**
* Whether this binding is on a structural template.
* The kind of template targeted by the binding, or null if this binding does not target a
* template.
*/
isStructuralTemplate: boolean;
templateKind: TemplateKind|null;

/**
* The i18n context, if this is an i18n attribute.
Expand All @@ -444,7 +455,8 @@ export interface AttributeOp extends Op<UpdateOp> {
*/
export function createAttributeOp(
target: XrefId, name: string, expression: o.Expression|Interpolation,
securityContext: SecurityContext, isTextAttribute: boolean, isStructuralTemplate: boolean,
securityContext: SecurityContext, isTextAttribute: boolean,
isStructuralTemplateAttribute: boolean, templateKind: TemplateKind|null,
i18nMessage: i18n.Message|null, sourceSpan: ParseSourceSpan): AttributeOp {
return {
kind: OpKind.Attribute,
Expand All @@ -454,7 +466,8 @@ export function createAttributeOp(
securityContext,
sanitizer: null,
isTextAttribute,
isStructuralTemplate,
isStructuralTemplateAttribute,
templateKind,
i18nContext: null,
i18nMessage,
sourceSpan,
Expand Down Expand Up @@ -510,7 +523,8 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
targetSlot: SlotHandle;

/**
* The main test expression (for a switch), or `null` (for an if, which has no test expression).
* The main test expression (for a switch), or `null` (for an if, which has no test
* expression).
*/
test: o.Expression|null;

Expand All @@ -527,8 +541,8 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
processed: o.Expression|null;

/**
* Control flow conditionals can accept a context value (this is a result of specifying an alias).
* This expression will be passed to the conditional instruction's context parameter.
* Control flow conditionals can accept a context value (this is a result of specifying an
* alias). This expression will be passed to the conditional instruction's context parameter.
*/
contextValue: o.Expression|null;

Expand Down Expand Up @@ -626,8 +640,8 @@ export function createDeferWhenOp(
/**
* An op that represents an expression in an i18n message.
*
* TODO: This can represent expressions used in both i18n attributes and normal i18n content. We may
* want to split these into two different op types, deriving from the same base class.
* TODO: This can represent expressions used in both i18n attributes and normal i18n content. We
* may want to split these into two different op types, deriving from the same base class.
*/
export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
DependsOnSlotContextOpTrait {
Expand All @@ -641,11 +655,11 @@ export interface I18nExpressionOp extends Op<UpdateOp>, ConsumesVarsTrait,
/**
* The Xref of the op that we need to `advance` to.
*
* In an i18n block, this is initially the i18n start op, but will eventually correspond to the
* final slot consumer in the owning i18n block.
* TODO: We should make text i18nExpressions target the i18nEnd instruction, instead the last slot
* consumer in the i18n block. This makes them resilient to that last consumer being deleted. (Or
* new slot consumers being added!)
* In an i18n block, this is initially the i18n start op, but will eventually correspond to
* the final slot consumer in the owning i18n block.
* TODO: We should make text i18nExpressions target the i18nEnd instruction, instead the last
* slot consumer in the i18n block. This makes them resilient to that last consumer being
* deleted. (Or new slot consumers being added!)
*
* In an i18n attribute, this is the xref of the corresponding elementStart/element.
*/
Expand Down Expand Up @@ -733,9 +747,9 @@ export interface I18nApplyOp extends Op<UpdateOp> {
owner: XrefId;

/**
* A handle for the slot that i18n apply instruction should apply to. In an i18n block, this is
* the slot of the i18n block this expression belongs to. In an i18n attribute, this is the slot
* of the corresponding i18nAttributes instruction.
* A handle for the slot that i18n apply instruction should apply to. In an i18n block, this
* is the slot of the i18n block this expression belongs to. In an i18n attribute, this is the
* slot of the corresponding i18nAttributes instruction.
*/
handle: SlotHandle;

Expand Down
30 changes: 18 additions & 12 deletions packages/compiler/src/template/pipeline/src/ingest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,15 @@ export function ingestHostProperty(
job.root.xref, bindingKind, property.name, expression, null,
SecurityContext
.NONE /* TODO: what should we pass as security context? Passing NONE for now. */,
isTextAttribute, false, /* TODO: How do Host bindings handle i18n attrs? */ null,
isTextAttribute, false, null, /* TODO: How do Host bindings handle i18n attrs? */ null,
property.sourceSpan));
}

export function ingestHostAttribute(
job: HostBindingCompilationJob, name: string, value: o.Expression): void {
const attrBinding = ir.createBindingOp(
job.root.xref, ir.BindingKind.Attribute, name, value, null, SecurityContext.NONE, true, false,
null,
/* TODO */ null,
/* TODO: host attribute source spans */ null!);
job.root.update.push(attrBinding);
Expand Down Expand Up @@ -165,7 +166,7 @@ function ingestElement(unit: ViewCompilationUnit, element: t.Element): void {
element.startSourceSpan);
unit.create.push(startOp);

ingestBindings(unit, startOp, element);
ingestBindings(unit, startOp, element, null);
ingestReferences(startOp, element);

// Start i18n, if needed, goes after the element create and bindings, but before the nodes
Expand Down Expand Up @@ -220,7 +221,7 @@ function ingestTemplate(unit: ViewCompilationUnit, tmpl: t.Template): void {
i18nPlaceholder, tmpl.startSourceSpan);
unit.create.push(templateOp);

ingestBindings(unit, templateOp, tmpl);
ingestBindings(unit, templateOp, tmpl, templateKind);
ingestReferences(templateOp, tmpl);
ingestNodes(childView, tmpl.children);

Expand Down Expand Up @@ -251,7 +252,7 @@ function ingestContent(unit: ViewCompilationUnit, content: t.Content): void {
for (const attr of content.attributes) {
ingestBinding(
unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, attr.i18n);
SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, null, attr.i18n);
}
unit.create.push(op);
}
Expand Down Expand Up @@ -754,7 +755,8 @@ function isPlainTemplate(tmpl: t.Template) {
* to their IR representation.
*/
function ingestBindings(
unit: ViewCompilationUnit, op: ir.ElementOpBase, element: t.Element|t.Template): void {
unit: ViewCompilationUnit, op: ir.ElementOpBase, element: t.Element|t.Template,
templateKind: ir.TemplateKind|null): void {
let flags: BindingFlags = BindingFlags.None;
let hasI18nAttributes = false;

Expand All @@ -771,12 +773,12 @@ function ingestBindings(
ingestBinding(
unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
SecurityContext.NONE, attr.sourceSpan, templateAttrFlags | BindingFlags.TextValue,
attr.i18n);
templateKind, attr.i18n);
hasI18nAttributes ||= attr.i18n !== undefined;
} else {
ingestBinding(
unit, op.xref, attr.name, attr.value, attr.type, attr.unit, attr.securityContext,
attr.sourceSpan, templateAttrFlags, attr.i18n);
attr.sourceSpan, templateAttrFlags, templateKind, attr.i18n);
hasI18nAttributes ||= attr.i18n !== undefined;
}
}
Expand All @@ -788,13 +790,14 @@ function ingestBindings(
// `BindingType.Attribute`.
ingestBinding(
unit, op.xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
SecurityContext.NONE, attr.sourceSpan, flags | BindingFlags.TextValue, attr.i18n);
SecurityContext.NONE, attr.sourceSpan, flags | BindingFlags.TextValue, templateKind,
attr.i18n);
hasI18nAttributes ||= attr.i18n !== undefined;
}
for (const input of element.inputs) {
ingestBinding(
unit, op.xref, input.name, input.value, input.type, input.unit, input.securityContext,
input.sourceSpan, flags, input.i18n);
input.sourceSpan, flags, templateKind, input.i18n);
hasI18nAttributes ||= input.i18n !== undefined;
}

Expand Down Expand Up @@ -888,7 +891,8 @@ enum BindingFlags {
function ingestBinding(
view: ViewCompilationUnit, xref: ir.XrefId, name: string, value: e.AST|o.Expression,
type: e.BindingType, unit: string|null, securityContext: SecurityContext,
sourceSpan: ParseSourceSpan, flags: BindingFlags, i18nMeta: i18n.I18nMeta|undefined): void {
sourceSpan: ParseSourceSpan, flags: BindingFlags, templateKind: ir.TemplateKind|null,
i18nMeta: i18n.I18nMeta|undefined): void {
if (value instanceof e.ASTWithSource) {
value = value.ast;
}
Expand Down Expand Up @@ -926,7 +930,8 @@ function ingestBinding(
const kind: ir.BindingKind = BINDING_KINDS.get(type)!;
view.update.push(ir.createBindingOp(
xref, kind, name, expression, unit, securityContext, !!(flags & BindingFlags.TextValue),
!!(flags & BindingFlags.IsStructuralTemplateAttribute), i18nMeta ?? null, sourceSpan));
!!(flags & BindingFlags.IsStructuralTemplateAttribute), templateKind, i18nMeta ?? null,
sourceSpan));
}

/**
Expand Down Expand Up @@ -1026,7 +1031,8 @@ function ingestControlFlowInsertionPoint(
for (const attr of root.attributes) {
ingestBinding(
unit, xref, attr.name, o.literal(attr.value), e.BindingType.Attribute, null,
SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, attr.i18n);
SecurityContext.NONE, attr.sourceSpan, BindingFlags.TextValue, ir.TemplateKind.Block,
attr.i18n);
}

const tagName = root instanceof t.Element ? root.name : root.tagName;
Expand Down
9 changes: 6 additions & 3 deletions packages/compiler/src/template/pipeline/src/phases/attribute_extraction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,18 @@ export function extractAttributes(job: CompilationJob): void {
for (const op of unit.ops()) {
switch (op.kind) {
case ir.OpKind.Attribute:
debugger;
extractAttributeOp(unit, op, elements);
break;
case ir.OpKind.Property:
debugger;
if (!op.isAnimationTrigger) {
let bindingKind: ir.BindingKind;
if (op.i18nMessage !== null) {
if (op.i18nMessage !== null && op.templateKind === null) {
// If the binding has an i18n context, it is an i18n attribute, and should have that
// kind in the consts array.
bindingKind = ir.BindingKind.I18n;
} else if (op.isStructuralTemplate) {
} else if (op.isStructuralTemplateAttribute) {
// TODO: How do i18n attributes on templates work?!
bindingKind = ir.BindingKind.Template;
} else {
Expand Down Expand Up @@ -119,7 +121,8 @@ function extractAttributeOp(

if (extractable) {
const extractedAttributeOp = ir.createExtractedAttributeOp(
op.target, op.isStructuralTemplate ? ir.BindingKind.Template : ir.BindingKind.Attribute,
op.target,
op.isStructuralTemplateAttribute ? ir.BindingKind.Template : ir.BindingKind.Attribute,
op.name, op.expression, op.i18nContext, op.i18nMessage);
if (unit.job.kind === CompilationJobKind.Host) {
// This attribute will apply to the enclosing host binding compilation unit, so order doesn't
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler/src/template/pipeline/src/phases/binding_specialization.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ export function specializeBindings(job: CompilationJob): void {
op,
ir.createAttributeOp(
op.target, op.name, op.expression, op.securityContext, op.isTextAttribute,
op.isStructuralTemplate, op.i18nMessage, op.sourceSpan));
op.isStructuralTemplateAttribute, op.templateKind, op.i18nMessage,
op.sourceSpan));
}
break;
case ir.BindingKind.Property:
Expand All @@ -65,8 +66,8 @@ export function specializeBindings(job: CompilationJob): void {
op,
ir.createPropertyOp(
op.target, op.name, op.expression, op.bindingKind === ir.BindingKind.Animation,
op.securityContext, op.isStructuralTemplate, op.i18nContext, op.i18nMessage,
op.sourceSpan));
op.securityContext, op.isStructuralTemplateAttribute, op.templateKind,
op.i18nContext, op.i18nMessage, op.sourceSpan));
}

break;
Expand Down
1 change: 1 addition & 0 deletions packages/compiler/src/template/pipeline/src/phases/const_collection.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ class ElementAttributes {
return;
}
this.known.add(name);
// TODO: Can this be its own phase
if (name === 'ngProjectAs') {
if (value === null || !(value instanceof o.LiteralExpr) || (value.value == null) ||
(typeof value.value?.toString() !== 'string')) {
Expand Down

0 comments on commit dc730d5

Please sign in to comment.