From 8628826535233ba5bc6b973cef860355b4c41931 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Sat, 31 Jul 2021 12:22:59 +0200 Subject: [PATCH] fix(core): incorrect error reported when trying to re-create view which had an error during creation (#43005) Currently if a view throws an error during creation mode, we mark it as `incompleteFirstPass` so that we can try to recover later. The recovery is only possible inside component views. The problem is that when this was introduced, I forgot to flip the `firstCreatePass` when an error is thrown which meant that calling `renderView` on the same component again is allowed. It will eventually hit an assertion which can be confusing for the end user. This issue only manifests itself when rendering views "manually" through `ViewContainerRef` (e.g. using `NgIf`). These changes flip the `firstCreatePass` back to false on errors so that trying to re-render the same view will throw an error which is consistent to the one that broke the view during creation. Fixes #41383. PR Close #43005 --- .../size-tracking/integration-payloads.json | 2 +- .../core/src/render3/instructions/shared.ts | 1 + .../test/acceptance/view_insertion_spec.ts | 43 ++++++++++++++++++- 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index b5c279699f..4b64276d0d 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1190, - "main-es2015": 136546, + "main-es2015": 137055, "polyfills-es2015": 37641 } } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 34d6ef6daf..a9214c4d73 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -336,6 +336,7 @@ export function renderView(tView: TView, lView: LView, context: T): void { // an error, mark the view as corrupted so we can try to recover. if (tView.firstCreatePass) { tView.incompleteFirstPass = true; + tView.firstCreatePass = false; } throw error; diff --git a/packages/core/test/acceptance/view_insertion_spec.ts b/packages/core/test/acceptance/view_insertion_spec.ts index 5d20dc20f8..72a424c9d2 100644 --- a/packages/core/test/acceptance/view_insertion_spec.ts +++ b/packages/core/test/acceptance/view_insertion_spec.ts @@ -7,7 +7,7 @@ */ import {CommonModule} from '@angular/common'; -import {ChangeDetectorRef, Component, ComponentFactoryResolver, Directive, EmbeddedViewRef, Injector, Input, NgModule, TemplateRef, ViewChild, ViewContainerRef, ViewRef} from '@angular/core'; +import {ChangeDetectorRef, Component, ComponentFactoryResolver, Directive, EmbeddedViewRef, Injectable, Injector, Input, NgModule, TemplateRef, ViewChild, ViewContainerRef, ViewRef} from '@angular/core'; import {TestBed} from '@angular/core/testing'; import {By} from '@angular/platform-browser'; import {onlyInIvy} from '@angular/private/testing'; @@ -886,5 +886,46 @@ describe('view insertion', () => { fixture.detectChanges(); expect(fixture.nativeElement.textContent).toContain('2'); }); + + it('should consistently report errors raised by createEmbeddedView', () => { + // Intentionally hasn't been added to `providers` so that it throws a DI error. + @Injectable() + class DoesNotExist { + } + + @Directive({selector: 'dir'}) + class Dir { + constructor(willCauseError: DoesNotExist) {} + } + + @Component({ + template: ` + + + + `, + }) + class App { + @ViewChild('broken') template !: TemplateRef; + + constructor(private _viewContainerRef: ViewContainerRef) {} + + insertTemplate() { + this._viewContainerRef.createEmbeddedView(this.template); + } + } + + TestBed.configureTestingModule({declarations: [App, Dir]}); + const fixture = TestBed.createComponent(App); + const tryRender = () => { + fixture.componentInstance.insertTemplate(); + fixture.detectChanges(); + }; + fixture.detectChanges(); + + // We try to render the same template twice to ensure that we get consistent error messages. + expect(tryRender).toThrowError(/No provider for DoesNotExist/); + expect(tryRender).toThrowError(/No provider for DoesNotExist/); + }); }); });