Skip to content

Commit 9980532

Browse files
mmalerbaalxhub
mmalerba
authored and
alxhub
committed
refactor(compiler): Split the i18n placeholder mega-phase (angular#52390)
The i18n placeholder resolution phase has accumulated too much logic, making it difficult to understand. This commit refactors it into several smaller phases to make it easier to manage. I suspect this will undergo further refactoring in the near future as I work through the ICU logic. In particular `ExtractedMessageOp` feels like a bit of a grab bag of properties, and the i18n const collection phase is also starting to get quite heavy. This refactor at least feels like a good start. PR Close angular#52390
1 parent e92d87e commit 9980532

File tree

11 files changed

+671
-564
lines changed

11 files changed

+671
-564
lines changed

‎packages/compiler/src/template/pipeline/ir/src/enums.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,3 +479,31 @@ export enum I18nParamResolutionTime {
479479
*/
480480
Postproccessing
481481
}
482+
483+
/**
484+
* Flags that describe what an i18n param value. These determine how the value is serialized into
485+
* the final map.
486+
*/
487+
export enum I18nParamValueFlags {
488+
None = 0b0000,
489+
490+
/**
491+
* This value represtents an element tag.
492+
*/
493+
ElementTag = 0b001,
494+
495+
/**
496+
* This value represents a template tag.
497+
*/
498+
TemplateTag = 0b0010,
499+
500+
/**
501+
* This value represents the opening of a tag.
502+
*/
503+
OpenTag = 0b0100,
504+
505+
/**
506+
* This value represents the closing of a tag.
507+
*/
508+
CloseTag = 0b1000,
509+
}

‎packages/compiler/src/template/pipeline/ir/src/expression.ts‎

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -960,12 +960,6 @@ export function transformExpressionsInOp(
960960
op.expression =
961961
op.expression && transformExpressionsInExpression(op.expression, transform, flags);
962962
break;
963-
case OpKind.ExtractedMessage:
964-
op.expression = transformExpressionsInExpression(op.expression, transform, flags);
965-
for (const statement of op.statements) {
966-
transformExpressionsInStatement(statement, transform, flags);
967-
}
968-
break;
969963
case OpKind.RepeaterCreate:
970964
op.track = transformExpressionsInExpression(op.track, transform, flags);
971965
if (op.trackByFn !== null) {
@@ -975,34 +969,31 @@ export function transformExpressionsInOp(
975969
case OpKind.Repeater:
976970
op.collection = transformExpressionsInExpression(op.collection, transform, flags);
977971
break;
978-
case OpKind.I18n:
979-
case OpKind.I18nStart:
980-
for (const [placeholder, expression] of op.params) {
981-
op.params.set(placeholder, transformExpressionsInExpression(expression, transform, flags));
982-
}
983-
break;
972+
case OpKind.Advance:
973+
case OpKind.Container:
974+
case OpKind.ContainerEnd:
975+
case OpKind.ContainerStart:
984976
case OpKind.Defer:
985-
case OpKind.DeferSecondaryBlock:
986977
case OpKind.DeferOn:
987-
case OpKind.Projection:
988-
case OpKind.ProjectionDef:
978+
case OpKind.DeferSecondaryBlock:
979+
case OpKind.DisableBindings:
989980
case OpKind.Element:
990-
case OpKind.ElementStart:
991981
case OpKind.ElementEnd:
992-
case OpKind.I18nEnd:
993-
case OpKind.Container:
994-
case OpKind.ContainerStart:
995-
case OpKind.ContainerEnd:
996-
case OpKind.Template:
997-
case OpKind.DisableBindings:
982+
case OpKind.ElementStart:
998983
case OpKind.EnableBindings:
999-
case OpKind.Text:
1000-
case OpKind.Pipe:
1001-
case OpKind.Advance:
1002-
case OpKind.Namespace:
984+
case OpKind.ExtractedMessage:
985+
case OpKind.I18n:
1003986
case OpKind.I18nApply:
987+
case OpKind.I18nEnd:
988+
case OpKind.I18nStart:
1004989
case OpKind.Icu:
1005990
case OpKind.IcuUpdate:
991+
case OpKind.Namespace:
992+
case OpKind.Pipe:
993+
case OpKind.Projection:
994+
case OpKind.ProjectionDef:
995+
case OpKind.Template:
996+
case OpKind.Text:
1006997
// These operations contain no expressions.
1007998
break;
1008999
default:

‎packages/compiler/src/template/pipeline/ir/src/ops/create.ts‎

Lines changed: 64 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import * as i18n from '../../../../../i18n/i18n_ast';
1010
import * as o from '../../../../../output/output_ast';
1111
import {ParseSourceSpan} from '../../../../../parse_util';
12-
import {BindingKind, DeferSecondaryKind, OpKind} from '../enums';
12+
import {BindingKind, DeferSecondaryKind, I18nParamValueFlags, OpKind} from '../enums';
1313
import {Op, OpList, XrefId} from '../operations';
1414
import {ConsumesSlotOpTrait, HasConstTrait, TRAIT_CONSUMES_SLOT, TRAIT_HAS_CONST, TRAIT_USES_SLOT_INDEX, UsesSlotIndexTrait} from '../traits';
1515

@@ -739,6 +739,27 @@ export function createDeferOnOp(xref: XrefId, sourceSpan: ParseSourceSpan): Defe
739739
};
740740
}
741741

742+
/**
743+
* Represents a single value in an i18n param map. Each placeholder in the map may have multiple of
744+
* these values associated with it.
745+
*/
746+
export interface I18nParamValue {
747+
/**
748+
* The value.
749+
*/
750+
value: string|number;
751+
752+
/**
753+
* The sub-template index associated with the value.
754+
*/
755+
subTemplateIndex: number|null;
756+
757+
/**
758+
* Flags associated with the value.
759+
*/
760+
flags: I18nParamValueFlags;
761+
}
762+
742763
/**
743764
* Represents an i18n message that has been extracted for inclusion in the consts array.
744765
*/
@@ -751,26 +772,61 @@ export interface ExtractedMessageOp extends Op<CreateOp> {
751772
owner: XrefId;
752773

753774
/**
754-
* The message expression.
775+
* The i18n message represented by this op.
776+
*/
777+
message: i18n.Message;
778+
779+
/**
780+
* Whether this op represents a root message (as opposed to a partial message for a sub-template
781+
* in a root message).
782+
*/
783+
isRoot: boolean;
784+
785+
/**
786+
* The param map, with placeholders represented as an array of value objects for easy
787+
* manipulation.
788+
*/
789+
params: Map<string, I18nParamValue[]>;
790+
791+
/**
792+
* The post-processing param map, with placeholders represented as an array of value objects for
793+
* easy manipulation.
794+
*/
795+
postprocessingParams: Map<string, I18nParamValue[]>;
796+
797+
/**
798+
* Whether this message needs post-processing.
799+
*/
800+
needsPostprocessing: boolean;
801+
802+
/**
803+
* The param map, with placeholders represented as an `Expression` to facilitate extraction to the
804+
* const arry.
755805
*/
756-
expression: o.Expression;
806+
formattedParams: Map<string, o.Expression>|null;
757807

758808
/**
759-
* The statements to construct the message.
809+
* The post-processing param map, with placeholders represented as an `Expression` to facilitate
810+
* extraction to the const arry.
760811
*/
761-
statements: o.Statement[];
812+
formattedPostprocessingParams: Map<string, o.Expression>|null;
762813
}
763814

764815
/**
765816
* Create an `ExtractedMessageOp`.
766817
*/
767818
export function createExtractedMessageOp(
768-
owner: XrefId, expression: o.Expression, statements: o.Statement[]): ExtractedMessageOp {
819+
owner: XrefId, message: i18n.Message, isRoot: boolean): ExtractedMessageOp {
769820
return {
770821
kind: OpKind.ExtractedMessage,
771822
owner,
772-
expression,
773-
statements,
823+
message,
824+
isRoot,
825+
params: new Map(),
826+
postprocessingParams: new Map(),
827+
needsPostprocessing: false,
828+
formattedParams: null,
829+
formattedPostprocessingParams: null,
774830
...NEW_OP,
775831
};
776832
}
@@ -794,17 +850,6 @@ export interface I18nOpBase extends Op<CreateOp>, ConsumesSlotOpTrait {
794850
*/
795851
message: i18n.Message;
796852

797-
/**
798-
* Map of values to use for named placeholders in the i18n message. (Resolved at message creation)
799-
*/
800-
params: Map<string, o.Expression>;
801-
802-
/**
803-
* Map of values to use for named placeholders in the i18n message. (Resolved during
804-
* post-porcessing)
805-
*/
806-
postprocessingParams: Map<string, o.Expression>;
807-
808853
/**
809854
* The index in the consts array where the message i18n message is stored.
810855
*/
@@ -814,11 +859,6 @@ export interface I18nOpBase extends Op<CreateOp>, ConsumesSlotOpTrait {
814859
* The index of this sub-block in the i18n message. For a root i18n block, this is null.
815860
*/
816861
subTemplateIndex: number|null;
817-
818-
/**
819-
* Whether the i18n message requires postprocessing.
820-
*/
821-
needsPostprocessing: boolean;
822862
}
823863

824864
/**
@@ -844,11 +884,8 @@ export function createI18nStartOp(xref: XrefId, message: i18n.Message, root?: Xr
844884
xref,
845885
root: root ?? xref,
846886
message,
847-
params: new Map(),
848-
postprocessingParams: new Map(),
849887
messageIndex: null,
850888
subTemplateIndex: null,
851-
needsPostprocessing: false,
852889
...NEW_OP,
853890
...TRAIT_CONSUMES_SLOT,
854891
};

‎packages/compiler/src/template/pipeline/src/emit.ts‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,15 @@ import {CompilationJob, CompilationJobKind as Kind, type ComponentCompilationJob
1616
import {phaseFindAnyCasts} from './phases/any_cast';
1717
import {phaseApplyI18nExpressions} from './phases/apply_i18n_expressions';
1818
import {phaseAssignI18nSlotDependencies} from './phases/assign_i18n_slot_dependencies';
19-
import {phaseCollapseSingletonInterpolations} from './phases/collapse_singleton_interpolations';
2019
import {phaseAttributeExtraction} from './phases/attribute_extraction';
2120
import {phaseBindingSpecialization} from './phases/binding_specialization';
2221
import {phaseChaining} from './phases/chaining';
22+
import {phaseCollapseSingletonInterpolations} from './phases/collapse_singleton_interpolations';
2323
import {phaseConditionals} from './phases/conditionals';
2424
import {phaseConstCollection} from './phases/const_collection';
2525
import {phaseEmptyElements} from './phases/empty_elements';
2626
import {phaseExpandSafeReads} from './phases/expand_safe_reads';
27-
import {phaseRepeaterDerivedVars} from './phases/repeater_derived_vars';
27+
import {phaseFormatI18nParams} from './phases/format_i18n_params';
2828
import {phaseGenerateAdvance} from './phases/generate_advance';
2929
import {phaseGenerateProjectionDef} from './phases/generate_projection_def';
3030
import {phaseGenerateVariables} from './phases/generate_variables';
@@ -47,25 +47,28 @@ import {phaseRemoveContentSelectors} from './phases/phase_remove_content_selecto
4747
import {phasePipeCreation} from './phases/pipe_creation';
4848
import {phasePipeVariadic} from './phases/pipe_variadic';
4949
import {phasePropagateI18nBlocks} from './phases/propagate_i18n_blocks';
50+
import {phasePropagateI18nPlaceholders} from './phases/propagate_i18n_placeholders';
5051
import {phasePureFunctionExtraction} from './phases/pure_function_extraction';
5152
import {phasePureLiteralStructures} from './phases/pure_literal_structures';
5253
import {phaseReify} from './phases/reify';
5354
import {phaseRemoveEmptyBindings} from './phases/remove_empty_bindings';
55+
import {phaseRepeaterDerivedVars} from './phases/repeater_derived_vars';
5456
import {phaseResolveContexts} from './phases/resolve_contexts';
5557
import {phaseResolveDollarEvent} from './phases/resolve_dollar_event';
56-
import {phaseResolveI18nPlaceholders} from './phases/resolve_i18n_placeholders';
58+
import {phaseResolveI18nElementPlaceholders} from './phases/resolve_i18n_element_placeholders';
59+
import {phaseResolveI18nExpressionPlaceholders} from './phases/resolve_i18n_expression_placeholders';
5760
import {phaseResolveNames} from './phases/resolve_names';
5861
import {phaseResolveSanitizers} from './phases/resolve_sanitizers';
5962
import {phaseSaveRestoreView} from './phases/save_restore_view';
6063
import {phaseSlotAllocation} from './phases/slot_allocation';
6164
import {phaseStyleBindingSpecialization} from './phases/style_binding_specialization';
6265
import {phaseTemporaryVariables} from './phases/temporary_variables';
66+
import {phaseTrackFnGeneration} from './phases/track_fn_generation';
67+
import {phaseTrackFnOptimization} from './phases/track_fn_optimization';
68+
import {phaseTrackVariables} from './phases/track_variables';
6369
import {phaseVarCounting} from './phases/var_counting';
6470
import {phaseVariableOptimization} from './phases/variable_optimization';
6571
import {phaseWrapIcus} from './phases/wrap_icus';
66-
import {phaseTrackVariables} from './phases/track_variables';
67-
import {phaseTrackFnGeneration} from './phases/track_fn_generation';
68-
import {phaseTrackFnOptimization} from './phases/track_fn_optimization';
6972

7073
type Phase = {
7174
fn: (job: CompilationJob) => void; kind: Kind.Both | Kind.Host | Kind.Tmpl;
@@ -114,9 +117,12 @@ const phases: Phase[] = [
114117
{kind: Kind.Both, fn: phaseExpandSafeReads},
115118
{kind: Kind.Both, fn: phaseTemporaryVariables},
116119
{kind: Kind.Tmpl, fn: phaseSlotAllocation},
117-
{kind: Kind.Tmpl, fn: phaseResolveI18nPlaceholders},
118-
{kind: Kind.Tmpl, fn: phaseTrackFnGeneration},
119120
{kind: Kind.Tmpl, fn: phaseI18nMessageExtraction},
121+
{kind: Kind.Tmpl, fn: phaseResolveI18nElementPlaceholders},
122+
{kind: Kind.Tmpl, fn: phaseResolveI18nExpressionPlaceholders},
123+
{kind: Kind.Tmpl, fn: phasePropagateI18nPlaceholders},
124+
{kind: Kind.Tmpl, fn: phaseFormatI18nParams},
125+
{kind: Kind.Tmpl, fn: phaseTrackFnGeneration},
120126
{kind: Kind.Tmpl, fn: phaseI18nConstCollection},
121127
{kind: Kind.Tmpl, fn: phaseConstTraitCollection},
122128
{kind: Kind.Both, fn: phaseConstCollection},

0 commit comments

Comments
 (0)