Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy): TestBed should tolerate synchronous use of compileComponents #28350

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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