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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ngIf else statement always re-assign template reference variable when component's change detection occur #14873

Closed
leo6104 opened this issue Mar 2, 2017 · 9 comments · Fixed by #15066
Assignees
Labels
area: core Issues related to the framework runtime regression Indicates than the issue relates to something that worked in a previous version

Comments

@leo6104
Copy link
Contributor

leo6104 commented Mar 2, 2017

I'm submitting a ... (check one with "x")

[x] bug report => search github for a similar issue or PR before submitting
[ ] feature request
[ ] support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

Current behavior

In my angular 4 hybrid web app, i tried to upgrade from 4.0.0-beta.8 to 4.0.0-rc.1 (also i tried 4.0.0-rc2). When upgraded, *ngIf else statement make component which is inside ng-template reload.

Expected behavior

template reference variable should be re-assign when *ngIf condition's variable changed.

Minimal reproduction of the problem with instructions

http://plnkr.co/edit/dXcEXDcYKDnyvNs0DwBZ

@Component({
  selector: 'angular-test',
  template: `
    <div *ngIf="test; else postBlock;">
      test
    </div>
    
    <ng-template #postBlock>
      <angular-test2></angular-test2>
</ng-template>
    `
})
export class TestComponent implements OnInit {
  private test:boolean;

  ngOnInit(): void {

  }
}


@Component({
  selector: 'angular-test2',
  template: `
tsetse
    `
})
export class Test2Component implements OnInit {
  private test:boolean;

  ngOnInit(): void {
    console.log('test2');
  }
}

<angular-test></angular-test> 

angular-test component's ng-template #postBlock makes angular-test2 component reload (console.log always occur when screen touch or timer or some component has changed)

What is the motivation / use case for changing the behavior?

In my app, post-detail component load post object from http request and then,
<div *ngIf="!post; else postBlock"></div> <ng-template #postBlock> ~~ <post-comment></post-comment> </ng-template>
code render #postBlock's inner html.
In post-comment component, it request http to getting comments in ngOnInit().
When i upgraded from beta.8 to rc.1/rc.2, post-comment's ngOnInit function call is infinitely occured.

Please tell us about your environment:

@angular/cli: 1.0.0-rc.0
node: 7.5.0
os: darwin x64
@angular/animations: 4.0.0-rc.2
@angular/cli: 1.0.0-rc.0
@angular/common: 4.0.0-rc.2
@angular/compiler: 4.0.0-rc.2
@angular/compiler-cli: 4.0.0-rc.2
@angular/core: 4.0.0-rc.2
@angular/forms: 4.0.0-rc.2
@angular/http: 4.0.0-rc.2
@angular/material: 2.0.0-beta.2
@angular/platform-browser: 4.0.0-rc.2
@angular/platform-browser-dynamic: 4.0.0-rc.2
@angular/platform-server: 4.0.0-rc.2
@angular/router: 4.0.0-rc.2
@ngtools/webpack: 1.2.11

  • Angular version: 4.0.0-rc.1 or 4.0.0-rc.2
  • Browser: [all | Chrome XX | Firefox XX | IE XX | Safari XX | Mobile Chrome XX | Android X.X Web Browser | iOS XX Safari | iOS XX UIWebView | iOS XX WKWebView ]
  • Language: [all | TypeScript X.X | ES6/7 | ES5]
    TypeScript 2.2.1

  • Node (for AoT issues): node --version = 7.5.0

@DzmitryShylovich
Copy link
Contributor

please add a plunkr

@leo6104
Copy link
Contributor Author

leo6104 commented Mar 3, 2017

Minimal reproduction is here.
http://plnkr.co/edit/dXcEXDcYKDnyvNs0DwBZ

@DzmitryShylovich
Copy link
Contributor

DzmitryShylovich commented Mar 3, 2017

@leo6104 thanks

The root cause here is that nodeValue recreates a new template on every CD.
@tbosch is this intentional? If yes then we need to something with identity comparison inside checkBinding

@vicb vicb added area: core Issues related to the framework runtime regression Indicates than the issue relates to something that worked in a previous version labels Mar 4, 2017
@tbosch
Copy link
Contributor

tbosch commented Mar 7, 2017

@DzmitryShylovich we don't want to store the TemplateRef as every property that we add to the NodeData costs... I think the best way forward is the following: Detect variables that are constant. E.g. #... if it is really just the variable reference is constant. This will also benefit our performance in general a bit...

@DzmitryShylovich
Copy link
Contributor

DzmitryShylovich commented Mar 7, 2017

@tbosch there's a similar issue with object literals in templates #13407 (comment) :

[ngOutletContext]="{ data: someData}"

this will create a new object everytime someData changes thus triggering ngTemplateOutlet's ngOnChanges. is this intentional as well?

@tbosch
Copy link
Contributor

tbosch commented Mar 7, 2017 via email

@insidewhy
Copy link

It would be a shame if the first angular release containing ngElse shipped with these semantics. Here we ported a bunch of code from ngSwitch to ngElse, and then ported it back when we hit this issue ;)

@tbosch
Copy link
Contributor

tbosch commented Mar 7, 2017

To be clear this bug is legit and we will fix it, only the comment above is works as intended.

tbosch added a commit to tbosch/angular that referenced this issue Mar 10, 2017
This was a regression introduced in v4 rc.0.

Fixes angular#14873
tbosch added a commit to tbosch/angular that referenced this issue Mar 10, 2017
This was a regression introduced in v4 rc.0.

Fixes angular#14873
tbosch added a commit to tbosch/angular that referenced this issue Mar 10, 2017
This was a regression introduced in v4 rc.0.

Fixes angular#14873
tbosch added a commit to tbosch/angular that referenced this issue Mar 10, 2017
This was a regression introduced in v4 rc.0.

Fixes angular#14873
insidewhy added a commit to insidewhy/collaborative-playlist that referenced this issue Mar 12, 2017
chuckjaz pushed a commit that referenced this issue Mar 13, 2017
SamVerschueren pushed a commit to SamVerschueren/angular that referenced this issue Mar 18, 2017
asnowwolf pushed a commit to asnowwolf/angular that referenced this issue Aug 11, 2017
juleskremer pushed a commit to juleskremer/angular that referenced this issue Aug 28, 2017
@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 Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime regression Indicates than the issue relates to something that worked in a previous version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants