Skip to content
Permalink
Browse files

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
  • Loading branch information...
AndrewKushnir authored and matsko committed Sep 6, 2019
1 parent 664e001 commit a1beba4b6e3b6ca020765aa6be01ce31eb35be54
Showing with 71 additions and 1 deletion.
  1. +6 −0 packages/core/src/render3/instructions/shared.ts
  2. +65 −1 packages/core/test/acceptance/di_spec.ts
@@ -471,6 +471,8 @@ export function renderComponentOrTemplate<T>(
const rendererFactory = hostView[RENDERER_FACTORY];
const normalExecutionPath = !getCheckNoChangesMode();
const creationModeIsActive = isCreationMode(hostView);
const previousOrParentTNode = getPreviousOrParentTNode();
const isParent = getIsParent();
try {
if (normalExecutionPath && !creationModeIsActive && rendererFactory.begin) {
rendererFactory.begin();
@@ -484,6 +486,7 @@ export function renderComponentOrTemplate<T>(
if (normalExecutionPath && !creationModeIsActive && rendererFactory.end) {
rendererFactory.end();
}
setPreviousOrParentTNode(previousOrParentTNode, isParent);
}
}

@@ -1642,6 +1645,8 @@ export function tickRootContext(rootContext: RootContext) {

export function detectChangesInternal<T>(view: LView, context: T) {
const rendererFactory = view[RENDERER_FACTORY];
const previousOrParentTNode = getPreviousOrParentTNode();
const isParent = getIsParent();

if (rendererFactory.begin) rendererFactory.begin();
try {
@@ -1652,6 +1657,7 @@ export function detectChangesInternal<T>(view: LView, context: T) {
throw error;
} finally {
if (rendererFactory.end) rendererFactory.end();
setPreviousOrParentTNode(previousOrParentTNode, isParent);
}
}

@@ -7,11 +7,12 @@
*/

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';
importINJECTOR_SCOPE} from '@angular/core/src/core';
import {ViewRef} from '@angular/core/src/render3/view_ref';
import {TestBed} from '@angular/core/testing';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
import {BehaviorSubject} from 'rxjs';

describe('di', () => {
describe('no dependencies', () => {
@@ -1118,6 +1119,69 @@ describe('di', () => {
// the nativeElement should be a comment
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', () => {

0 comments on commit a1beba4

Please sign in to comment.
You can’t perform that action at this time.