From af2d22c43d7dcbd45b061a8ed03afa9391a82fcf Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Mon, 25 Nov 2019 16:39:31 -0800 Subject: [PATCH] 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 --- packages/core/src/render3/i18n.ts | 20 ++-- packages/core/test/acceptance/i18n_spec.ts | 104 +++++++++++++++++++++ 2 files changed, 115 insertions(+), 9 deletions(-) diff --git a/packages/core/src/render3/i18n.ts b/packages/core/src/render3/i18n.ts index e4a6d313d8..e4f4366dba 100644 --- a/packages/core/src/render3/i18n.ts +++ b/packages/core/src/render3/i18n.ts @@ -12,7 +12,7 @@ import {SRCSET_ATTRS, URI_ATTRS, VALID_ATTRS, VALID_ELEMENTS, getTemplateContent import {InertBodyHelper} from '../sanitization/inert_body'; import {_sanitizeUrl, sanitizeSrcset} from '../sanitization/url_sanitizer'; import {addAllToArray} from '../util/array_utils'; -import {assertDataInRange, assertDefined, assertEqual, assertGreaterThan} from '../util/assert'; +import {assertDataInRange, assertDefined, assertEqual} from '../util/assert'; import {bindingUpdated} from './bindings'; import {attachPatchData} from './context_discovery'; @@ -190,7 +190,6 @@ function parseICUBlock(pattern: string): IcuExpression { } } - assertGreaterThan(cases.indexOf('other'), -1, 'Missing key "other" in ICU statement.'); // TODO(ocombe): support ICU expressions in attributes, see #21615 return {type: icuType, mainBinding: mainBinding, cases, values}; } @@ -895,18 +894,21 @@ function readUpdateOpCodes( // Update the active caseIndex const caseIndex = getCaseIndex(tIcu, value); icuTNode.activeCaseIndex = caseIndex !== -1 ? caseIndex : null; - - // Add the nodes for the new case - readCreateOpCodes(-1, tIcu.create[caseIndex], viewData); - caseCreated = true; + if (caseIndex > -1) { + // Add the nodes for the new case + readCreateOpCodes(-1, tIcu.create[caseIndex], viewData); + caseCreated = true; + } break; case I18nUpdateOpCode.IcuUpdate: tIcuIndex = updateOpCodes[++j] as number; tIcu = icus ![tIcuIndex]; icuTNode = getTNode(nodeIndex, viewData) as TIcuContainerNode; - readUpdateOpCodes( - tIcu.update[icuTNode.activeCaseIndex !], icus, bindingsStartIndex, changeMask, - viewData, caseCreated); + if (icuTNode.activeCaseIndex !== null) { + readUpdateOpCodes( + tIcu.update[icuTNode.activeCaseIndex], icus, bindingsStartIndex, changeMask, + viewData, caseCreated); + } break; } } diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index bd01f90502..049a966bf2 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -1094,6 +1094,110 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { fixture.detectChanges(); expect(fixture.debugElement.nativeElement.innerHTML).toContain('plus d\'un'); }); + + it('should support ICUs without "other" cases', () => { + loadTranslations({ + idA: '{VAR_SELECT, select, 1 {un (select)} 2 {deux (select)}}', + idB: '{VAR_PLURAL, plural, =1 {un (plural)} =2 {deux (plural)}}', + }); + + @Component({ + selector: 'app', + template: ` +
{count, select, 1 {one (select)} 2 {two (select)}}
- +
{count, plural, =1 {one (plural)} =2 {two (plural)}}
+ ` + }) + class AppComponent { + count = 1; + } + + TestBed.configureTestingModule({declarations: [AppComponent]}); + + const fixture = TestBed.createComponent(AppComponent); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)'); + + fixture.componentInstance.count = 3; + fixture.detectChanges(); + // there is no ICU case for count=3 + expect(fixture.nativeElement.textContent.trim()).toBe('-'); + + fixture.componentInstance.count = 4; + fixture.detectChanges(); + // there is no ICU case for count=4, making sure content is still empty + expect(fixture.nativeElement.textContent.trim()).toBe('-'); + + fixture.componentInstance.count = 2; + fixture.detectChanges(); + // check switching to an existing case after processing an ICU without matching case + expect(fixture.nativeElement.textContent.trim()).toBe('deux (select) - deux (plural)'); + + fixture.componentInstance.count = 1; + fixture.detectChanges(); + // check that we can go back to the first ICU case + expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)'); + }); + + it('should support nested ICUs without "other" cases', () => { + loadTranslations({ + idA: '{VAR_SELECT_1, select, A {{VAR_SELECT, select, ' + + '1 {un (select)} 2 {deux (select)}}} other {}}', + idB: '{VAR_SELECT, select, A {{VAR_PLURAL, plural, ' + + '=1 {un (plural)} =2 {deux (plural)}}} other {}}', + }); + + @Component({ + selector: 'app', + template: ` +
{ + type, select, + A {{count, select, 1 {one (select)} 2 {two (select)}}} + other {} + }
- +
{ + type, select, + A {{count, plural, =1 {one (plural)} =2 {two (plural)}}} + other {} + }
+ ` + }) + class AppComponent { + type = 'A'; + count = 1; + } + + TestBed.configureTestingModule({declarations: [AppComponent]}); + + const fixture = TestBed.createComponent(AppComponent); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)'); + + fixture.componentInstance.count = 3; + fixture.detectChanges(); + // there is no case for count=3 in nested ICU + expect(fixture.nativeElement.textContent.trim()).toBe('-'); + + fixture.componentInstance.count = 4; + fixture.detectChanges(); + // there is no case for count=4 in nested ICU, making sure content is still empty + expect(fixture.nativeElement.textContent.trim()).toBe('-'); + + fixture.componentInstance.count = 2; + fixture.detectChanges(); + // check switching to an existing case after processing nested ICU without matching case + expect(fixture.nativeElement.textContent.trim()).toBe('deux (select) - deux (plural)'); + + fixture.componentInstance.count = 1; + fixture.detectChanges(); + // check that we can go back to the first case in nested ICU + expect(fixture.nativeElement.textContent).toBe('un (select) - un (plural)'); + + fixture.componentInstance.type = 'B'; + fixture.detectChanges(); + // check that nested ICU is removed if root ICU case has changed + expect(fixture.nativeElement.textContent.trim()).toBe('-'); + }); }); describe('should support attributes', () => {