Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plural expression within ng-container with ngIf prevents following markup from being rendered #38144

Closed
anton-github opened this issue Jul 20, 2020 · 3 comments
Labels
area: i18n freq2: medium P2 The issue is important to a large percentage of users, with a workaround regression Indicates than the issue relates to something that worked in a previous version state: confirmed type: bug/fix
Milestone

Comments

@anton-github
Copy link

anton-github commented Jul 20, 2020

馃悶 bug report

Affected Package

It seems like any angular version with Ivy enabled is affected.

Is this a regression?

Since Ivy is enabled by default in the newer versions I would say yes, it's a regression.

Description

Plurals ICU expressions prevent following markup from being rendered when used within ng-container with ngIf on it. For version 10 it's true only for subsequent renderings, see examples in the next section.
Short code example:

  <ng-container *ngIf="true">
    you have {{ 1 }} {1, plural, one {task} other {tasks}} to do
  </ng-container>

On subsequent renderings the code above will render you have 1 task

馃敩 Minimal Reproduction

Toggle block visibility by clicking button to see how the result is changed

https://stackblitz.com/edit/plural-ng8 - all is OK
https://stackblitz.com/edit/plural-ng9rc1 - is OK with Ivy disabled, otherwise the behavior is wrong for both first and subsequent renderings
https://stackblitz.com/edit/plural-ng10 - is OK with Ivy disabled, otherwise the behavior is wrong only for subsequent renderings

馃敟 Exception or Error

Nothing is logged

馃實 Your Environment

Angular Version:
Please see stackblitz examples dependencies above

@petebacondarwin petebacondarwin added area: i18n engine: ivy freq2: medium regression Indicates than the issue relates to something that worked in a previous version type: bug/fix labels Jul 20, 2020
@ngbot ngbot bot added this to the Backlog milestone Jul 20, 2020
@petebacondarwin
Copy link
Member

It appears that in the 傻傻text() instruction for the " to do " string, the call to appendChild() fails because it is not able to find a renderParent - (getRenderParent() returns null).

@petebacondarwin
Copy link
Member

Actually that is a redherring, since at the point the appendChild() is called, this node is not yet attached to the node tree, since it is part of an ngIf, which has not yet been applied.

On further digging, I see that on the first rendering, the tNode.next for the ICU does have a reference to the text node, but on the second rendering this property is null. So we need to work out why the ICU tNode is losing the link to the following text tNode on second rendering.

@jelbourn jelbourn added the P2 The issue is important to a large percentage of users, with a workaround label Oct 1, 2020
mhevery added a commit to mhevery/angular that referenced this issue Oct 9, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 12, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 12, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 12, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 12, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 12, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 12, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 13, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 15, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 16, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 16, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 17, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 20, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 20, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 20, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 20, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 21, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 21, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 21, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 21, 2020
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
mhevery added a commit to mhevery/angular that referenced this issue Oct 21, 2020
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
alxhub pushed a commit that referenced this issue Oct 22, 2020
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
@alxhub alxhub closed this as completed in ca11ef2 Oct 22, 2020
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: i18n freq2: medium P2 The issue is important to a large percentage of users, with a workaround regression Indicates than the issue relates to something that worked in a previous version state: confirmed type: bug/fix
Projects
None yet
Development

No branches or pull requests

3 participants