fix(ivy): don't increment expandoStartIndex after directives are matched (#28424)
i18n instructions create text nodes dynamically and save them between bindings and the expando block in `LView`. e.g., they try to create the following order in `LView`. ``` | -- elements -- | -- bindings -- | -- dynamic i18n text -- | -- expando (dirs, injectors) -- | ``` Each time a new text node is created, it is pushed to the end of the array and the `expandoStartIndex` marker is incremented, so the section begins slightly later. This happens in `allocExpando`. This is fine if no directives have been created yet. The end of the array will be in the "dynamic text node" section. | -- elements -- | -- bindings -- | -- dynamic i18n text -- | However, this approach doesn't work if dynamic text nodes are created after directives are matched (for example when the directive uses host bindings). In that case, there are already directives and injectors saved in the "expando" section. So pushing to the end of `LView` actually pushes after the expando section. What we get is this: ``` | -- elements -- | -- bindings -- | -- dynamic i18n text -- | -- expando -- | -- dynamic i18n text-- | ``` In this case, the `expandoStartIndex` shouldn't be incremented because we are not inserting anything before the expando section (it's now after the expando section). But because it is incremented in the code right now, it's now pointing to an index in the middle of the expando section. This PR fixes that so that we only increment the `expandoStartIndex` if nothing was pushed into the expando section. FW-978 #resolve PR Close #28424
This commit is contained in:
Olivier Combe
committed by
Matias Niemelä
Matias Niemelä
parent
3f73dfa151
commit
baf103c98f
@ -8,14 +8,13 @@
|
||||
|
||||
import {noop} from '../../../compiler/src/render3/view/util';
|
||||
import {Component as _Component} from '../../src/core';
|
||||
import {defineComponent} from '../../src/render3/definition';
|
||||
import {defineComponent, defineDirective} from '../../src/render3/definition';
|
||||
import {getTranslationForTemplate, i18n, i18nApply, i18nAttributes, i18nEnd, i18nExp, i18nPostprocess, i18nStart} from '../../src/render3/i18n';
|
||||
import {RenderFlags} from '../../src/render3/interfaces/definition';
|
||||
import {AttributeMarker} from '../../src/render3/interfaces/node';
|
||||
import {getNativeByIndex} from '../../src/render3/util';
|
||||
|
||||
import {NgIf} from './common_with_def';
|
||||
|
||||
import {element, elementEnd, elementStart, template, text, nextContext, bind, elementProperty, projectionDef, projection, elementContainerStart, elementContainerEnd} from '../../src/render3/instructions';
|
||||
import {allocHostVars, element, elementEnd, elementStart, template, text, nextContext, bind, elementProperty, projectionDef, projection, elementContainerStart, elementContainerEnd} from '../../src/render3/instructions';
|
||||
import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nUpdateOpCode, I18nUpdateOpCodes, TI18n} from '../../src/render3/interfaces/i18n';
|
||||
import {HEADER_OFFSET, LView, TVIEW} from '../../src/render3/interfaces/view';
|
||||
import {ComponentFixture, TemplateFixture} from './render_util';
|
||||
@ -83,7 +82,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 1,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
'simple text', nbConsts,
|
||||
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild
|
||||
@ -105,7 +103,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 5,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
'Hello ',
|
||||
nbConsts,
|
||||
@ -142,7 +139,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 1,
|
||||
expandoStartIndex: nbConsts,
|
||||
create:
|
||||
['', nbConsts, index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild],
|
||||
update: [
|
||||
@ -164,7 +160,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 1,
|
||||
expandoStartIndex: nbConsts,
|
||||
create:
|
||||
['', nbConsts, index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild],
|
||||
update: [
|
||||
@ -198,7 +193,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 2,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
'',
|
||||
nbConsts,
|
||||
@ -229,7 +223,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 2,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
spanElement << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
|
||||
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild,
|
||||
@ -257,7 +250,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 1,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
bElement << I18nMutateOpCode.SHIFT_REF | I18nMutateOpCode.Select,
|
||||
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild,
|
||||
@ -291,7 +283,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 5,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
COMMENT_MARKER, 'ICU 1', icuCommentNodeIndex,
|
||||
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild
|
||||
@ -308,7 +299,6 @@ describe('Runtime i18n', () => {
|
||||
icus: [{
|
||||
type: 1,
|
||||
vars: [4, 3, 3],
|
||||
expandoStartIndex: icuCommentNodeIndex + 1,
|
||||
childIcus: [[], [], []],
|
||||
cases: ['0', '1', 'other'],
|
||||
create: [
|
||||
@ -407,7 +397,6 @@ describe('Runtime i18n', () => {
|
||||
|
||||
expect(opCodes).toEqual({
|
||||
vars: 6,
|
||||
expandoStartIndex: nbConsts,
|
||||
create: [
|
||||
COMMENT_MARKER, 'ICU 1', icuCommentNodeIndex,
|
||||
index << I18nMutateOpCode.SHIFT_PARENT | I18nMutateOpCode.AppendChild
|
||||
@ -425,7 +414,6 @@ describe('Runtime i18n', () => {
|
||||
{
|
||||
type: 0,
|
||||
vars: [1, 1, 1],
|
||||
expandoStartIndex: lastTextNodeIndex + 1,
|
||||
childIcus: [[], [], []],
|
||||
cases: ['cat', 'dog', 'other'],
|
||||
create: [
|
||||
@ -455,7 +443,6 @@ describe('Runtime i18n', () => {
|
||||
{
|
||||
type: 1,
|
||||
vars: [1, 4],
|
||||
expandoStartIndex: icuCommentNodeIndex + 1,
|
||||
childIcus: [[], [0]],
|
||||
cases: ['0', 'other'],
|
||||
create: [
|
||||
@ -1350,6 +1337,105 @@ describe('Runtime i18n', () => {
|
||||
expect(fixture.html).toEqual(`<div title="start 2 middle 1 end">trad 1</div>`);
|
||||
});
|
||||
|
||||
it('should work with directives and host bindings', () => {
|
||||
let directiveInstances: Directive[] = [];
|
||||
|
||||
class Directive {
|
||||
// @HostBinding('className')
|
||||
klass = 'foo';
|
||||
|
||||
static ngDirectiveDef = defineDirective({
|
||||
type: Directive,
|
||||
selectors: [['', 'dir', '']],
|
||||
factory: () => {
|
||||
const instance = new Directive();
|
||||
directiveInstances.push(instance);
|
||||
return instance;
|
||||
},
|
||||
hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => {
|
||||
if (rf & RenderFlags.Create) {
|
||||
allocHostVars(1);
|
||||
}
|
||||
if (rf & RenderFlags.Update) {
|
||||
elementProperty(elementIndex, 'className', bind(ctx.klass), null, true);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Translated template:
|
||||
// <div i18n [test]="false" i18n-title title="start {{exp2}} middle {{exp1}} end">
|
||||
// trad {<7B>0<EFBFBD>, plural,
|
||||
// =0 {no <b title="none">emails</b>!}
|
||||
// =1 {one <i>email</i>}
|
||||
// other {<7B>0<EFBFBD> emails}
|
||||
// }
|
||||
// </div>
|
||||
|
||||
const MSG_DIV_1 = `trad {<7B>0<EFBFBD>, plural,
|
||||
=0 {no <b title="none">emails</b>!}
|
||||
=1 {one <i>email</i>}
|
||||
other {<7B>0<EFBFBD> emails}
|
||||
}`;
|
||||
const MSG_DIV_1_ATTR_1 = ['title', `start <20>1<EFBFBD> middle <20>0<EFBFBD> end`];
|
||||
|
||||
class MyApp {
|
||||
exp1 = 1;
|
||||
exp2 = 2;
|
||||
|
||||
static ngComponentDef = defineComponent({
|
||||
type: MyApp,
|
||||
selectors: [['my-app']],
|
||||
factory: () => new MyApp(),
|
||||
consts: 6,
|
||||
vars: 5,
|
||||
directives: [Directive],
|
||||
template: (rf: RenderFlags, ctx: MyApp) => {
|
||||
if (rf & RenderFlags.Create) {
|
||||
elementStart(0, 'div', [AttributeMarker.SelectOnly, 'dir']);
|
||||
{
|
||||
i18nAttributes(1, MSG_DIV_1_ATTR_1);
|
||||
i18nStart(2, MSG_DIV_1);
|
||||
{
|
||||
elementStart(3, 'b', [AttributeMarker.SelectOnly, 'dir']); // Will be removed
|
||||
{ i18nAttributes(4, MSG_DIV_1_ATTR_1); }
|
||||
elementEnd();
|
||||
}
|
||||
i18nEnd();
|
||||
}
|
||||
elementEnd();
|
||||
element(5, 'div', [AttributeMarker.SelectOnly, 'dir']);
|
||||
}
|
||||
if (rf & RenderFlags.Update) {
|
||||
i18nExp(bind(ctx.exp1));
|
||||
i18nExp(bind(ctx.exp2));
|
||||
i18nApply(1);
|
||||
i18nExp(bind(ctx.exp1));
|
||||
i18nApply(2);
|
||||
i18nExp(bind(ctx.exp1));
|
||||
i18nExp(bind(ctx.exp2));
|
||||
i18nApply(4);
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
const fixture = new ComponentFixture(MyApp);
|
||||
// the "test" attribute should not be reflected in the DOM as it is here only for directive
|
||||
// matching purposes
|
||||
expect(fixture.html)
|
||||
.toEqual(
|
||||
`<div class="foo" title="start 2 middle 1 end">trad one <i>email</i><!--ICU 23--></div><div class="foo"></div>`);
|
||||
|
||||
directiveInstances.forEach(instance => instance.klass = 'bar');
|
||||
fixture.component.exp1 = 2;
|
||||
fixture.component.exp2 = 3;
|
||||
fixture.update();
|
||||
expect(fixture.html)
|
||||
.toEqual(
|
||||
`<div class="bar" title="start 3 middle 2 end">trad 2 emails<!--ICU 23--></div><div class="bar"></div>`);
|
||||
});
|
||||
|
||||
describe('projection', () => {
|
||||
it('should project the translations', () => {
|
||||
@Component({selector: 'child', template: '<p><ng-content></ng-content></p>'})
|
||||
|
||||
Reference in New Issue
Block a user