Skip to content

Commit 781003f

Browse files
AndrewKushnirmatsko
authored andcommitted
fix(ivy): support ICUs without "other" cases (#34042)
Prior to this commit, there was a runtime check in i18n logic to make sure "other" case is always present in an ICU. That was not a requirement in View Engine, so ICUs that previously worked may produce errors. This commit removes that restriction and adds support for ICUs without "other" cases. PR Close #34042
1 parent cbc28f2 commit 781003f

File tree

2 files changed

+115
-9
lines changed

2 files changed

+115
-9
lines changed

packages/core/src/render3/i18n.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS, getTemplateContent
1212
import {InertBodyHelper} from '../sanitization/inert_body';
1313
import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer';
1414
import {addAllToArray} from '../util/array_utils';
15-
import {assertDataInRange, assertDefined, assertEqual, assertGreaterThan} from '../util/assert';
15+
import {assertDataInRange, assertDefined, assertEqual} from '../util/assert';
1616

1717
import {bindingUpdated} from './bindings';
1818
import {attachPatchData} from './context_discovery';
@@ -190,7 +190,6 @@ function parseICUBlock(pattern: string): IcuExpression {
190190
}
191191
}
192192

193-
assertGreaterThan(cases.indexOf('other'), -1, 'Missing key "other" in ICU statement.');
194193
// TODO(ocombe): support ICU expressions in attributes, see #21615
195194
return {type: icuType, mainBinding: mainBinding, cases, values};
196195
}
@@ -895,18 +894,21 @@ function readUpdateOpCodes(
895894
// Update the active caseIndex
896895
const caseIndex = getCaseIndex(tIcu, value);
897896
icuTNode.activeCaseIndex = caseIndex !== -1 ? caseIndex : null;
898-
899-
// Add the nodes for the new case
900-
readCreateOpCodes(-1, tIcu.create[caseIndex], viewData);
901-
caseCreated = true;
897+
if (caseIndex > -1) {
898+
// Add the nodes for the new case
899+
readCreateOpCodes(-1, tIcu.create[caseIndex], viewData);
900+
caseCreated = true;
901+
}
902902
break;
903903
case I18nUpdateOpCode.IcuUpdate:
904904
tIcuIndex = updateOpCodes[++j] as number;
905905
tIcu = icus ![tIcuIndex];
906906
icuTNode = getTNode(nodeIndex, viewData) as TIcuContainerNode;
907-
readUpdateOpCodes(
908-
tIcu.update[icuTNode.activeCaseIndex !], icus, bindingsStartIndex, changeMask,
909-
viewData, caseCreated);
907+
if (icuTNode.activeCaseIndex !== null) {
908+
readUpdateOpCodes(
909+
tIcu.update[icuTNode.activeCaseIndex], icus, bindingsStartIndex, changeMask,
910+
viewData, caseCreated);
911+
}
910912
break;
911913
}
912914
}

packages/core/test/acceptance/i18n_spec.ts

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1094,6 +1094,110 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => {
10941094
fixture.detectChanges();
10951095
expect(fixture.debugElement.nativeElement.innerHTML).toContain('plus d\'un');
10961096
});
1097+
1098+
it('should support ICUs without "other" cases', () => {
1099+
loadTranslations({
1100+
idA: '{VAR_SELECT, select, 1 {un (select)} 2 {deux (select)}}',
1101+
idB: '{VAR_PLURAL, plural, =1 {un (plural)} =2 {deux (plural)}}',
1102+
});
1103+
1104+
@Component({
1105+
selector: 'app',
1106+
template: `
1107+
<div i18n="@@idA">{count, select, 1 {one (select)} 2 {two (select)}}</div> -
1108+
<div i18n="@@idB">{count, plural, =1 {one (plural)} =2 {two (plural)}}</div>
1109+
`
1110+
})
1111+
class AppComponent {
1112+
count = 1;
1113+
}
1114+
1115+
TestBed.configureTestingModule({declarations: [AppComponent]});
1116+
1117+
const fixture = TestBed.createComponent(AppComponent);
1118+
fixture.detectChanges();
1119+
expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)');
1120+
1121+
fixture.componentInstance.count = 3;
1122+
fixture.detectChanges();
1123+
// there is no ICU case for count=3
1124+
expect(fixture.nativeElement.textContent.trim()).toBe('-');
1125+
1126+
fixture.componentInstance.count = 4;
1127+
fixture.detectChanges();
1128+
// there is no ICU case for count=4, making sure content is still empty
1129+
expect(fixture.nativeElement.textContent.trim()).toBe('-');
1130+
1131+
fixture.componentInstance.count = 2;
1132+
fixture.detectChanges();
1133+
// check switching to an existing case after processing an ICU without matching case
1134+
expect(fixture.nativeElement.textContent.trim()).toBe('deux (select) - deux (plural)');
1135+
1136+
fixture.componentInstance.count = 1;
1137+
fixture.detectChanges();
1138+
// check that we can go back to the first ICU case
1139+
expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)');
1140+
});
1141+
1142+
it('should support nested ICUs without "other" cases', () => {
1143+
loadTranslations({
1144+
idA: '{VAR_SELECT_1, select, A {{VAR_SELECT, select, ' +
1145+
'1 {un (select)} 2 {deux (select)}}} other {}}',
1146+
idB: '{VAR_SELECT, select, A {{VAR_PLURAL, plural, ' +
1147+
'=1 {un (plural)} =2 {deux (plural)}}} other {}}',
1148+
});
1149+
1150+
@Component({
1151+
selector: 'app',
1152+
template: `
1153+
<div i18n="@@idA">{
1154+
type, select,
1155+
A {{count, select, 1 {one (select)} 2 {two (select)}}}
1156+
other {}
1157+
}</div> -
1158+
<div i18n="@@idB">{
1159+
type, select,
1160+
A {{count, plural, =1 {one (plural)} =2 {two (plural)}}}
1161+
other {}
1162+
}</div>
1163+
`
1164+
})
1165+
class AppComponent {
1166+
type = 'A';
1167+
count = 1;
1168+
}
1169+
1170+
TestBed.configureTestingModule({declarations: [AppComponent]});
1171+
1172+
const fixture = TestBed.createComponent(AppComponent);
1173+
fixture.detectChanges();
1174+
expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)');
1175+
1176+
fixture.componentInstance.count = 3;
1177+
fixture.detectChanges();
1178+
// there is no case for count=3 in nested ICU
1179+
expect(fixture.nativeElement.textContent.trim()).toBe('-');
1180+
1181+
fixture.componentInstance.count = 4;
1182+
fixture.detectChanges();
1183+
// there is no case for count=4 in nested ICU, making sure content is still empty
1184+
expect(fixture.nativeElement.textContent.trim()).toBe('-');
1185+
1186+
fixture.componentInstance.count = 2;
1187+
fixture.detectChanges();
1188+
// check switching to an existing case after processing nested ICU without matching case
1189+
expect(fixture.nativeElement.textContent.trim()).toBe('deux (select) - deux (plural)');
1190+
1191+
fixture.componentInstance.count = 1;
1192+
fixture.detectChanges();
1193+
// check that we can go back to the first case in nested ICU
1194+
expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)');
1195+
1196+
fixture.componentInstance.type = 'B';
1197+
fixture.detectChanges();
1198+
// check that nested ICU is removed if root ICU case has changed
1199+
expect(fixture.nativeElement.textContent.trim()).toBe('-');
1200+
});
10971201
});
10981202

10991203
describe('should support attributes', () => {

0 commit comments

Comments
 (0)