Duplicate text when using ICU expression in list without 'other' #38073

sodaDreamer · 2020-07-15T09:27:35Z

🐞 bug report

Affected Package

The issue is caused by package @angular/....

Is this a regression?

Pretty sure this wasn't an issue in Angular 7. Noticed the issue after updating to Angular 9.

Description

When using an ICU select expression inside a list, if you do not include the "other" placeholder, then subsequent changes to an item in the list causes the replacement text to display twice.

Issue #34018 suggests that the "other" case should actually be required but this is now no longer enforced. It might be worth reinstating the warning if this is deemed not to be a bug.

🔬 Minimal Reproduction

https://stackblitz.com/edit/angular-ivy-cuoymq?file=src%2Fapp%2Fapp.component.html

🌍 Your Environment

Angular Version:


Angular CLI: 9.1.10
Node: 12.18.2
OS: win32 x64

Angular: 9.1.11
... animations, common, compiler, compiler-cli, core, forms
... language-service, localize, platform-browser
... platform-browser-dynamic, router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.10
@angular-devkit/build-angular     0.901.10
@angular-devkit/build-optimizer   0.901.10
@angular-devkit/build-webpack     0.901.10
@angular-devkit/core              9.1.10
@angular-devkit/schematics        9.1.10
@angular/cli                      9.1.10
@ngtools/webpack                  9.1.10
@schematics/angular               9.1.10
@schematics/update                0.901.10
rxjs                              6.6.0
typescript                        3.8.3
webpack                           4.42.0

The text was updated successfully, but these errors were encountered:

mhevery · 2020-07-17T17:22:15Z

First let me say thank you for creating simple and easy to follow reproduction with text explaining what to click and what is about to happen. KUDOS!

Yes, this seems to be a bug. While ICU may require other I don't see a reason why that should affect the runtime algorithm.

Fixes an issue where the previous value of an ICU expression wasn't being removed, if it didn't have an `other` parameter and it's the last value inside a template. The issue seems to be due to the fact that we were resetting the index of the active expression if we didn't manage to match a value to it which prevented the previous node from being cleaned up on the next template run. Fixes angular#38073.

Fixes an issue where the previous value of an ICU expression wasn't being removed, if it didn't have an `other` parameter and it's the last value inside a template. The issue seems to be due to the fact that we were resetting the index of the active expression if we didn't manage to match a value to it which prevented the previous node from being cleaned up on the next template run. Also cleans up an optional parameter in one of the internal functions and adds stronger typing to the i18n unit tests. Fixes angular#38073.

Fixes an issue where the previous value of an ICU expression wasn't being removed, if it didn't have an `other` parameter and it's the last value inside a template. The issue is due to the `createDynamicNodeAtIndex` function which sets a new node at a particular index inside the `LView`, but it doesn't account for any node that may have existed there beforehand. Since we've overwritten the reference, we don't have a way of removing the node from the DOM anymore. Also cleans up an optional parameter in one of the internal functions and adds stronger typing to the i18n unit tests. Fixes angular#38073. fixup! fix(core): ICU expression previous value not removed inside template fixup! fix(core): ICU expression previous value not removed inside template

Fixes an issue where the previous value of an ICU expression wasn't being removed, if it didn't have an `other` parameter and it's the last value inside a template. The issue is due to the `createDynamicNodeAtIndex` function which sets a new node at a particular index inside the `LView`, but it doesn't account for any node that may have existed there beforehand. Since we've overwritten the reference, we don't have a way of removing the node from the DOM anymore. Also cleans up an optional parameter in one of the internal functions and adds stronger typing to the i18n unit tests. Fixes angular#38073.

Before this refactoring/fix the ICU would store the current selected index in `TView`. This is incorrect, since if ICU is in `ngFor` it will cause issues in some circumstances. This refactoring properly moves the state to `LView`. closes angular#37021 closes angular#38144 closes angular#38073

Before this refactoring/fix the ICU would store the current selected index in `TView`. This is incorrect, since if ICU is in `ngFor` it will cause issues in some circumstances. This refactoring properly moves the state to `LView`. closes #37021 closes #38144 closes #38073 PR Close #39233

angular-automatic-lock-bot · 2020-11-22T16:26:07Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

JoostK added the comp: i18n label Jul 15, 2020

ngbot bot added this to the needsTriage milestone Jul 15, 2020

petebacondarwin added core: DOM rendering state: confirmed type: bug/fix freq1: low regression labels Jul 15, 2020

ngbot bot removed this from the needsTriage milestone Jul 16, 2020

ngbot bot added this to the Backlog milestone Jul 16, 2020

pkozlowski-opensource removed the core: DOM rendering label Jul 16, 2020

crisbeto mentioned this issue Jul 18, 2020

fix(core): ICU expression previous value not removed inside template #38130

Closed

crisbeto self-assigned this Jul 18, 2020

crisbeto added the state: has PR label Jul 18, 2020

crisbeto removed their assignment Aug 11, 2020

crisbeto removed the state: has PR label Aug 11, 2020

jelbourn added the P2 label Oct 1, 2020

alxhub closed this in ca11ef2 Oct 22, 2020

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 22, 2020

Sep	OCT	Nov
	20
2020	2021	2022

angular / angular Public

Duplicate text when using ICU expression in list without 'other' #38073

Duplicate text when using ICU expression in list without 'other' #38073

sodaDreamer commented Jul 15, 2020 •

edited

mhevery commented Jul 17, 2020

angular-automatic-lock-bot bot commented Nov 22, 2020

angular / angular Public

Duplicate text when using ICU expression in list without 'other' #38073

Duplicate text when using ICU expression in list without 'other' #38073

Comments

sodaDreamer commented Jul 15, 2020 • edited

🐞 bug report

Affected Package

Is this a regression?

Description

🔬 Minimal Reproduction

🌍 Your Environment

mhevery commented Jul 17, 2020

angular-automatic-lock-bot bot commented Nov 22, 2020

sodaDreamer commented Jul 15, 2020 •

edited