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

KeyValueDiffers infinite loop when properties order change #14997

Closed
ghetolay opened this issue Mar 7, 2017 · 2 comments · Fixed by #15968
Closed

KeyValueDiffers infinite loop when properties order change #14997

ghetolay opened this issue Mar 7, 2017 · 2 comments · Fixed by #15968
Labels
area: common Issues related to APIs in the @angular/common package area: core Issues related to the framework runtime regression Indicates than the issue relates to something that worked in a previous version type: bug/fix

Comments

@ghetolay
Copy link
Contributor

ghetolay commented Mar 7, 2017

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

[x] bug report
[ ] feature request
[ ] support request

Current behavior
When you change an object by a new object with properties on a different order an infinite loop may occur.

Expected behavior
Nobody wants an infinite loop :)

Minimal reproduction of the problem with instructions
You can easily see it using ngStyle an swapping between :

{
  width: '10px',
  height: '20px'
};

and

{
  height: '30px',
  width: '20px'
};

Here is a plunker doing just that : https://plnkr.co/edit/nRYa2aNydsKOYS8eaDEq

This is affecting both 2.4.9 and 4.0.0-rc.2 (you can easily swap version on the plunker inside config.js).

I'll reference that previous issue #9115 which is different but still about KeyValueDiffers and properties order.

@ghetolay
Copy link
Contributor Author

ghetolay commented Mar 7, 2017

I've just look at the DefaultKeyValueDiffer code and it's scary :).
I'm assuming this is because it handles deep mutable object. This made me think, Angular don't want to force users to use immutable data and this is fine but this adds complexity when it comes to handle things like differs (ngFor, ngStyle, ngClass...).

Would not it be worth it to have multiple implementation for immutable and mutable data so those using immutable data (cause of OnPush, NgRx etc...) could benefit from lightweight implementation ?

I'm saying all that because I'm assuming DefaultKeyValueDiffer is so complex because of mutable data so I hope I'm not wrong here.

@IgorMinar IgorMinar added area: common Issues related to APIs in the @angular/common package area: core Issues related to the framework runtime type: bug/fix labels Mar 13, 2017
@pkozlowski-opensource pkozlowski-opensource added the regression Indicates than the issue relates to something that worked in a previous version label Apr 7, 2017
@vicb vicb added this to Prio2 in Compiler Apr 11, 2017
@vicb vicb moved this from Prio2 to Prio4 in Compiler Apr 11, 2017
vicb added a commit to vicb/angular that referenced this issue Apr 13, 2017
@vicb vicb removed this from Prio_col2 in Compiler Apr 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: common Issues related to APIs in the @angular/common package area: core Issues related to the framework runtime regression Indicates than the issue relates to something that worked in a previous version type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants