Skip to content

Commit

Permalink
fix(ivy): TestBed should tolerate synchronous use of compileComponents
Browse files Browse the repository at this point in the history
TestBed.compileComponents has always been an async API. However,
ViewEngine tolerated using this API in a synchronous manner if the
components declared in the testing module did not have any async
resources (templateUrl, styleUrls). This change makes the ivy TestBed
mirror this tolerance by configuring such components synchronously.

Ref: FW-992
  • Loading branch information
jelbourn committed Jan 24, 2019
1 parent 9f9024b commit b3f66c8
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 20 deletions.
6 changes: 3 additions & 3 deletions packages/core/src/metadata/resource_loading.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ export function maybeQueueResolutionOfComponentResources(metadata: Component) {
}
}

export function componentNeedsResolution(component: Component) {
return component.templateUrl || component.styleUrls && component.styleUrls.length;
export function componentNeedsResolution(component: Component): boolean {
return !!(component.templateUrl || component.styleUrls && component.styleUrls.length);
}
export function clearResolutionOfComponentResourcesQueue() {
componentResourceResolutionQueue.clear();
}

function unwrapResponse(response: string | {text(): Promise<string>}): string|Promise<string> {
return typeof response == 'string' ? response : response.text();
}
}
24 changes: 24 additions & 0 deletions packages/core/test/test_bed_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ export class InheritedCmp extends SimpleCmp {
export class SimpleApp {
}

@Component({selector: 'inline-template', template: '<p>Hello</p>'})
export class ComponentWithInlineTemplate {
}

@NgModule({
declarations: [HelloWorld, SimpleCmp, WithRefsCmp, InheritedCmp, SimpleApp],
imports: [GreetingModule],
Expand Down Expand Up @@ -194,6 +198,26 @@ describe('TestBed', () => {
expect(simpleApp.nativeElement).toHaveText('simple - inherited');
});

it('should resolve components without async resources synchronously', (done) => {
TestBed
.configureTestingModule({
declarations: [ComponentWithInlineTemplate],
})
.compileComponents()
.then(done)
.catch(error => {
// This should not throw any errors. If an error is thrown, the test will fail.
// Specifically use `catch` here to mark the test as done and *then* throw the error
// so that the test isn't treated as a timeout.
done();
throw error;
});

// Intentionally call `createComponent` before `compileComponents` is resolved. We want this to
// work for components that don't have any async resources (templateUrl, styleUrls).
TestBed.createComponent(ComponentWithInlineTemplate);
});

onlyInIvy('patched ng defs should be removed after resetting TestingModule')
.it('make sure we restore ng defs to their initial states', () => {
@Pipe({name: 'somePipe', pure: true})
Expand Down
45 changes: 28 additions & 17 deletions packages/core/testing/src/r3_test_bed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import {
// clang-format on
import {ResourceLoader} from '@angular/compiler';

import {clearResolutionOfComponentResourcesQueue, resolveComponentResources} from '../../src/metadata/resource_loading';
import {clearResolutionOfComponentResourcesQueue, componentNeedsResolution, resolveComponentResources,} from '../../src/metadata/resource_loading';
import {ComponentFixture} from './component_fixture';
import {MetadataOverride} from './metadata_override';
import {ComponentResolver, DirectiveResolver, NgModuleResolver, PipeResolver, Resolver} from './resolvers';
Expand Down Expand Up @@ -374,6 +374,8 @@ export class TestBedRender3 implements Injector, TestBed {
const declarations: Type<any>[] = flatten(this._declarations || EMPTY_ARRAY, resolveForwardRef);

const componentOverrides: [Type<any>, Component][] = [];
let hasAsyncResources = false;

// Compile the components declared by this module
declarations.forEach(declaration => {
const component = resolvers.component.resolve(declaration);
Expand All @@ -382,25 +384,34 @@ export class TestBedRender3 implements Injector, TestBed {
const metadata = {...component};
compileComponent(declaration, metadata);
componentOverrides.push([declaration, metadata]);
hasAsyncResources = hasAsyncResources || componentNeedsResolution(component);
}
});

let resourceLoader: ResourceLoader;

return resolveComponentResources(url => {
if (!resourceLoader) {
resourceLoader = this.compilerInjector.get(ResourceLoader);
}
return Promise.resolve(resourceLoader.get(url));
})
.then(() => {
componentOverrides.forEach((override: [Type<any>, Component]) => {
// Once resolved, we override the existing metadata, ensuring that the resolved
// resources
// are only available until the next TestBed reset (when `resetTestingModule` is called)
this.overrideComponent(override[0], {set: override[1]});
});
});
const overrideComponents = () => {
componentOverrides.forEach((override: [Type<any>, Component]) => {
// Override the existing metadata, ensuring that the resolved resources
// are only available until the next TestBed reset (when `resetTestingModule` is called)
this.overrideComponent(override[0], {set: override[1]});
});
};

// If the component has no async resources (templateUrl, styleUrls), we can finish
// synchronously. This is important so that users who mistakenly treat `compileComponents`
// as synchronous don't encounter an error, as ViewEngine was tolerant of this.
if (!hasAsyncResources) {
overrideComponents();
return Promise.resolve();
} else {
let resourceLoader: ResourceLoader;
return resolveComponentResources(url => {
if (!resourceLoader) {
resourceLoader = this.compilerInjector.get(ResourceLoader);
}
return Promise.resolve(resourceLoader.get(url));
})
.then(overrideComponents);
}
}

get(token: any, notFoundValue: any = Injector.THROW_IF_NOT_FOUND): any {
Expand Down

0 comments on commit b3f66c8

Please sign in to comment.