fix(ivy): restore global state after running refreshView (#32521)

Prior to this commit, the `previousOrParentTNode` was set to null after performing all operations within `refreshView` function. It's causing problems in more complex scenarios, for example when change detection is triggered during DI (see test added as a part of this commit). As a result, global state might be corrupted. This commit captures current value of `previousOrParentTNode` and restores it after `refreshView` call.

PR Close #32521
This commit is contained in:
Andrew Kushnir 2019-09-06 16:48:13 -07:00 committed by Matias Niemelä
parent 664e0015d4
commit a1beba4b6e
2 changed files with 71 additions and 1 deletions

View File

@ -471,6 +471,8 @@ export function renderComponentOrTemplate<T>(
const rendererFactory = hostView[RENDERER_FACTORY]; const rendererFactory = hostView[RENDERER_FACTORY];
const normalExecutionPath = !getCheckNoChangesMode(); const normalExecutionPath = !getCheckNoChangesMode();
const creationModeIsActive = isCreationMode(hostView); const creationModeIsActive = isCreationMode(hostView);
const previousOrParentTNode = getPreviousOrParentTNode();
const isParent = getIsParent();
try { try {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) { if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
rendererFactory.begin(); rendererFactory.begin();
@ -484,6 +486,7 @@ export function renderComponentOrTemplate<T>(
if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) { if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) {
rendererFactory.end(); rendererFactory.end();
} }
setPreviousOrParentTNode(previousOrParentTNode, isParent);
} }
} }
@ -1642,6 +1645,8 @@ export function tickRootContext(rootContext: RootContext) {
export function detectChangesInternal<T>(view: LView, context: T) { export function detectChangesInternal<T>(view: LView, context: T) {
const rendererFactory = view[RENDERER_FACTORY]; const rendererFactory = view[RENDERER_FACTORY];
const previousOrParentTNode = getPreviousOrParentTNode();
const isParent = getIsParent();
if (rendererFactory.begin) rendererFactory.begin(); if (rendererFactory.begin) rendererFactory.begin();
try { try {
@ -1652,6 +1657,7 @@ export function detectChangesInternal<T>(view: LView, context: T) {
throw error; throw error;
} finally { } finally {
if (rendererFactory.end) rendererFactory.end(); if (rendererFactory.end) rendererFactory.end();
setPreviousOrParentTNode(previousOrParentTNode, isParent);
} }
} }

View File

@ -7,11 +7,12 @@
*/ */
import {CommonModule} from '@angular/common'; import {CommonModule} from '@angular/common';
import {Attribute, ChangeDetectorRef, Component, Directive, ElementRef, EventEmitter, Host, HostBinding, INJECTOR, Inject, Injectable, InjectionToken, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, forwardRef, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID} from '@angular/core'; import {Attribute, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, ElementRef, EventEmitter, Host, HostBinding, INJECTOR, Inject, Injectable, InjectionToken, Injector, Input, LOCALE_ID, ModuleWithProviders, NgModule, NgZone, Optional, Output, Pipe, PipeTransform, Self, SkipSelf, TemplateRef, ViewChild, ViewContainerRef, forwardRef, ɵDEFAULT_LOCALE_ID as DEFAULT_LOCALE_ID} from '@angular/core';
import {ɵINJECTOR_SCOPE} from '@angular/core/src/core'; import {ɵINJECTOR_SCOPE} from '@angular/core/src/core';
import {ViewRef} from '@angular/core/src/render3/view_ref'; import {ViewRef} from '@angular/core/src/render3/view_ref';
import {TestBed} from '@angular/core/testing'; import {TestBed} from '@angular/core/testing';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing'; import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
import {BehaviorSubject} from 'rxjs';
describe('di', () => { describe('di', () => {
describe('no dependencies', () => { describe('no dependencies', () => {
@ -1118,6 +1119,69 @@ describe('di', () => {
// the nativeElement should be a comment // the nativeElement should be a comment
expect(directive.elementRef.nativeElement.nodeType).toEqual(Node.COMMENT_NODE); expect(directive.elementRef.nativeElement.nodeType).toEqual(Node.COMMENT_NODE);
}); });
it('should be available if used in conjunction with other tokens', () => {
@Injectable()
class ServiceA {
subject: any;
constructor(protected zone: NgZone) {
this.subject = new BehaviorSubject<any>(1);
// trigger change detection
zone.run(() => { this.subject.next(2); });
}
}
@Directive({selector: '[dir]'})
class DirectiveA {
constructor(public service: ServiceA, public elementRef: ElementRef) {}
}
@Component({
selector: 'child',
template: `<div id="test-id" dir></div>`,
})
class ChildComp {
@ViewChild(DirectiveA, {static: false}) directive !: DirectiveA;
}
@Component({
selector: 'root',
template: '...',
})
class RootComp {
public childCompRef !: ComponentRef<ChildComp>;
constructor(
public factoryResolver: ComponentFactoryResolver, public vcr: ViewContainerRef) {}
create() {
const factory = this.factoryResolver.resolveComponentFactory(ChildComp);
this.childCompRef = this.vcr.createComponent(factory);
this.childCompRef.changeDetectorRef.detectChanges();
}
}
// this module is needed, so that View Engine can generate factory for ChildComp
@NgModule({
declarations: [DirectiveA, RootComp, ChildComp],
entryComponents: [RootComp, ChildComp],
})
class ModuleA {
}
TestBed.configureTestingModule({
imports: [ModuleA],
providers: [ServiceA],
});
const fixture = TestBed.createComponent(RootComp);
fixture.autoDetectChanges();
fixture.componentInstance.create();
const {elementRef} = fixture.componentInstance.childCompRef.instance.directive;
expect(elementRef.nativeElement.id).toBe('test-id');
});
}); });
describe('TemplateRef', () => { describe('TemplateRef', () => {