fix(ngMessagesInclude): do not compile template if scope is destroyed

[jpekkala]

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix

What is the current behavior? (You can also link to an open issue here)
#12695

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?

Please check if the PR fulfills these requirements

• The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)

Other information:

Messages imported with ngMessagesInclude are loaded asynchronously and they can arrive after the ngMessages element has already been removed from DOM. Previously such messages were still compiled and that caused a $compile:ctreq error. Now they are silently ignored if ngMessagesInclude's scope has already been destroyed.

Closes #12695

[Narretz]

Instead of spying on compile, I'd rather see an expectation for not throwing, or a check that validates the DOM structure is as we expect it. The call to compile is a bit to implementation specific for me

[gkalpak]

return spyOn($delegate).and.callThrough(); is more straight-forward.

[jpekkala]

Thanks for the reviews @Narretz and @gkalpak. An expectation for not throwing was my first idea too but after seeing the analogous test in ngInclude, I decided to do it the same way as they had.

This is how I would write the test with the expectation for not throwing:

it('should not throw if scope has been destroyed when template request is ready',
  inject(function($rootScope, $httpBackend, $compile) {
    $httpBackend.expectGET('messages.html').respond('<div ng-message="a">A</div>');
    $rootScope.show = true;
    var html =
        '<div ng-if="show">' +
          '<div ng-messages="items">' +
            '<div ng-messages-include="messages.html"></div>' +
          '</div>' +
        '</div>';

    element = $compile(html)($rootScope);
    $rootScope.$digest();
    $rootScope.show = false;
    $rootScope.$digest();
    expect(function() {
      $httpBackend.flush();
    }).not.toThrow();
}));

Should I update the pull request?

[Narretz]

In that case, can you just add the not.toThroe expectation?