Skip to content

Commit f9bb53a

Browse files
AndrewKushnirbenlesh
authored andcommitted
fix(ivy): allow TestBed.createComponent to create components in isolation (angular#29981)
Prior to this change, components created via TestBed.createComponent in the same test were placed into the same root context, which caused problems in conjunction with fixture.autoDetectChanges usage in the same test. Specifically, change detection was triggered immediately for created component (starting from the 2nd one) even if it was not required/desired. This commit makes Ivy and VE behavior consistent: now every component created via TestBed.createComponent is isolated from each other. Current solution uses host element id naming convention, which is not ideal, but helps avoid public API surface changes at this point (we might revisit this approach later). Note: this commit also adds extra tests to verify bootstrap and change detection behavior in case of multiple components in `bootstrap` array in @NgModule, to make sure this behavior is aligned between Ivy and VE. PR Close angular#29981
1 parent c1d5fbd commit f9bb53a

File tree

4 files changed

+126
-4
lines changed

4 files changed

+126
-4
lines changed

packages/core/src/render3/component_ref.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,17 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
149149

150150
const rootFlags = this.componentDef.onPush ? LViewFlags.Dirty | LViewFlags.IsRoot :
151151
LViewFlags.CheckAlways | LViewFlags.IsRoot;
152-
const rootContext: RootContext =
153-
!isInternalRootView ? rootViewInjector.get(ROOT_CONTEXT) : createRootContext();
152+
153+
// Check whether this Component needs to be isolated from other components, i.e. whether it
154+
// should be placed into its own (empty) root context or existing root context should be used.
155+
// Note: this is internal-only convention and might change in the future, so it should not be
156+
// relied upon externally.
157+
const isIsolated = typeof rootSelectorOrNode === 'string' &&
158+
/^#root-ng-internal-isolated-\d+/.test(rootSelectorOrNode);
159+
160+
const rootContext: RootContext = (isInternalRootView || isIsolated) ?
161+
createRootContext() :
162+
rootViewInjector.get(ROOT_CONTEXT);
154163

155164
const renderer = rendererFactory.createRenderer(hostRNode, this.componentDef);
156165

packages/core/test/test_bed_spec.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, NgModule, Optional, Pipe, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵtext as text} from '@angular/core';
9+
import {Component, Directive, ErrorHandler, Inject, Injectable, InjectionToken, Input, NgModule, Optional, Pipe, ɵsetClassMetadata as setClassMetadata, ɵɵdefineComponent as defineComponent, ɵɵtext as text} from '@angular/core';
1010
import {TestBed, getTestBed} from '@angular/core/testing/src/test_bed';
1111
import {By} from '@angular/platform-browser';
1212
import {expect} from '@angular/platform-browser/testing/src/matchers';
@@ -257,6 +257,42 @@ describe('TestBed', () => {
257257
expect(simpleApp.nativeElement).toHaveText('simple - inherited');
258258
});
259259

260+
it('should not trigger change detection for ComponentA while calling TestBed.createComponent for ComponentB',
261+
() => {
262+
const log: string[] = [];
263+
@Component({
264+
selector: 'comp-a',
265+
template: '...',
266+
})
267+
class CompA {
268+
@Input() inputA: string = '';
269+
ngOnInit() { log.push('CompA:ngOnInit', this.inputA); }
270+
}
271+
272+
@Component({
273+
selector: 'comp-b',
274+
template: '...',
275+
})
276+
class CompB {
277+
@Input() inputB: string = '';
278+
ngOnInit() { log.push('CompB:ngOnInit', this.inputB); }
279+
}
280+
281+
TestBed.configureTestingModule({declarations: [CompA, CompB]});
282+
283+
log.length = 0;
284+
const appA = TestBed.createComponent(CompA);
285+
appA.componentInstance.inputA = 'a';
286+
appA.autoDetectChanges();
287+
expect(log).toEqual(['CompA:ngOnInit', 'a']);
288+
289+
log.length = 0;
290+
const appB = TestBed.createComponent(CompB);
291+
appB.componentInstance.inputB = 'b';
292+
appB.autoDetectChanges();
293+
expect(log).toEqual(['CompB:ngOnInit', 'b']);
294+
});
295+
260296
it('should resolve components without async resources synchronously', (done) => {
261297
TestBed
262298
.configureTestingModule({

packages/core/testing/src/r3_test_bed.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ export class TestBedRender3 implements Injector, TestBed {
343343

344344
createComponent<T>(type: Type<T>): ComponentFixture<T> {
345345
const testComponentRenderer: TestComponentRenderer = this.get(TestComponentRenderer);
346-
const rootElId = `root${_nextRootElementId++}`;
346+
const rootElId = `root-ng-internal-isolated-${_nextRootElementId++}`;
347347
testComponentRenderer.insertRootElement(rootElId);
348348

349349
const componentDef = (type as any).ngComponentDef;

packages/platform-browser/test/browser/bootstrap_spec.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -470,5 +470,82 @@ function bootstrap(
470470
});
471471
}));
472472

473+
describe('change detection', () => {
474+
const log: string[] = [];
475+
@Component({
476+
selector: 'hello-app',
477+
template: '<div id="button-a" (click)="onClick()">{{title}}</div>',
478+
})
479+
class CompA {
480+
title: string = '';
481+
ngDoCheck() { log.push('CompA:ngDoCheck'); }
482+
onClick() {
483+
this.title = 'CompA';
484+
log.push('CompA:onClick');
485+
}
486+
}
487+
488+
@Component({
489+
selector: 'hello-app-2',
490+
template: '<div id="button-b" (click)="onClick()">{{title}}</div>',
491+
})
492+
class CompB {
493+
title: string = '';
494+
ngDoCheck() { log.push('CompB:ngDoCheck'); }
495+
onClick() {
496+
this.title = 'CompB';
497+
log.push('CompB:onClick');
498+
}
499+
}
500+
501+
it('should be triggered for all bootstrapped components in case change happens in one of them',
502+
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
503+
@NgModule({
504+
imports: [BrowserModule],
505+
declarations: [CompA, CompB],
506+
bootstrap: [CompA, CompB],
507+
schemas: [CUSTOM_ELEMENTS_SCHEMA]
508+
})
509+
class TestModuleA {
510+
}
511+
platformBrowserDynamic().bootstrapModule(TestModuleA).then((ref) => {
512+
log.length = 0;
513+
el.querySelectorAll('#button-a')[0].click();
514+
expect(log).toContain('CompA:onClick');
515+
expect(log).toContain('CompA:ngDoCheck');
516+
expect(log).toContain('CompB:ngDoCheck');
517+
518+
log.length = 0;
519+
el2.querySelectorAll('#button-b')[0].click();
520+
expect(log).toContain('CompB:onClick');
521+
expect(log).toContain('CompA:ngDoCheck');
522+
expect(log).toContain('CompB:ngDoCheck');
523+
524+
async.done();
525+
});
526+
}));
527+
528+
529+
it('should work in isolation for each component bootstrapped individually',
530+
inject([AsyncTestCompleter], (async: AsyncTestCompleter) => {
531+
const refPromise1 = bootstrap(CompA);
532+
const refPromise2 = bootstrap(CompB);
533+
Promise.all([refPromise1, refPromise2]).then((refs) => {
534+
log.length = 0;
535+
el.querySelectorAll('#button-a')[0].click();
536+
expect(log).toContain('CompA:onClick');
537+
expect(log).toContain('CompA:ngDoCheck');
538+
expect(log).not.toContain('CompB:ngDoCheck');
539+
540+
log.length = 0;
541+
el2.querySelectorAll('#button-b')[0].click();
542+
expect(log).toContain('CompB:onClick');
543+
expect(log).toContain('CompB:ngDoCheck');
544+
expect(log).not.toContain('CompA:ngDoCheck');
545+
546+
async.done();
547+
});
548+
}));
549+
});
473550
});
474551
}

0 commit comments

Comments
 (0)