Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

ng-repeat should not leave junk directive fragments in the DOM #4930

Closed
MrHash opened this issue Nov 12, 2013 · 7 comments
Closed

ng-repeat should not leave junk directive fragments in the DOM #4930

MrHash opened this issue Nov 12, 2013 · 7 comments
Assignees
Milestone

Comments

@MrHash
Copy link

MrHash commented Nov 12, 2013

Demonstrating the problem - http://plnkr.co/edit/GBuCj8uQkpm0H5yG3nGk?p=preview

Note that the directive HTML fragments are left in the DOM although on screen the directives appear as expected, although sometimes the order of the directives is mixed up. You can use your DOM inspector to see the junk directive fragments.

Delaying the application to scope of the initial array using a timeout results in expected behaviour in the DOM and on screen. Use of resolve in a route does not solve the problem.

The implications for the former problem are that classes are not applied correctly and order is not consistent.

@IgorMinar
Copy link
Contributor

it's not clear from the description of this error, but it does appear that we are adding extra nodes with no content to the repeated elements. looks like an off by one error of some sort (maybe in block scanning algorithm).

I was able to repro this with v1.2.1

See screenshot with the extra node highlighted:

screen shot 2013-11-18 at 3 43 24 pm

@ghost ghost assigned tbosch Nov 22, 2013
@tbosch
Copy link
Contributor

tbosch commented Nov 26, 2013

Looking into this now.

@tbosch
Copy link
Contributor

tbosch commented Nov 26, 2013

Minimal case to reproduce: element that has a ng-repeat and a directive that uses a templateUrl.

@tbosch
Copy link
Contributor

tbosch commented Nov 26, 2013

Here is the line that is causing this: https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L1704

@tbosch
Copy link
Contributor

tbosch commented Nov 27, 2013

So the real problem is this:

  • first ng-repeat clones the element before the template has been loaded
  • then the template arrives, but it does not update the element reference that ng-repeat has and creates a new one on it's own.
  • by this, ng-repeat is not able to delete that new element.

Here is a test without ng-repeat that shows the problem:

iit('should update element clones after a directive with templateUrl arrives', inject(function($httpBackend, 
    $compile, $rootScope) {
  $compileProvider.directive('test', function() {
    return {
      templateUrl: 'test.html'
    };
  });
  $httpBackend.whenGET('test.html').respond('<span>hello</span>');
  element = jqLite('<div test></div>');
  var link = $compile(element);
  var cloneBeforeTemplateArrived = link($rootScope.$new(), angular.noop);

  $httpBackend.flush();
  dump(cloneBeforeTemplateArrived);
  expect(cloneBeforeTemplateArrived.text()).toBe('hello');
}));

@tbosch
Copy link
Contributor

tbosch commented Nov 27, 2013

So there are the two places that need to be fixed:

I don't understand the logic behind the startNode and endNode in details yet, so have to look into this more...

tbosch added a commit to tbosch/angular.js that referenced this issue Dec 6, 2013
…he cloning

If an element has a directive whose content is loaded using `templateUrl`,
and the element is cloned using a linking function before the template arrives,
the clone needs to be updated as well.

This also updates `ngIf` and `ngRepeat` to keep the connection to the clone
of a tranclude function, so that they know about the changes a directive with
`templateUrl` does to the element in the future.

Fixes to angular#4930.
@tbosch
Copy link
Contributor

tbosch commented Dec 6, 2013

Closed via b0972a2

@tbosch tbosch closed this as completed Dec 6, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
…he cloning

If an element has a directive whose content is loaded using `templateUrl`,
and the element is cloned using a linking function before the template arrives,
the clone needs to be updated as well.

This also updates `ngIf` and `ngRepeat` to keep the connection to the clone
of a tranclude function, so that they know about the changes a directive with
`templateUrl` does to the element in the future.

Fixes to angular#4930.
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
…he cloning

If an element has a directive whose content is loaded using `templateUrl`,
and the element is cloned using a linking function before the template arrives,
the clone needs to be updated as well.

This also updates `ngIf` and `ngRepeat` to keep the connection to the clone
of a tranclude function, so that they know about the changes a directive with
`templateUrl` does to the element in the future.

Fixes to angular#4930.
@lgalfaso lgalfaso mentioned this issue May 2, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants