fix(compiler): prevent race conditions
Previously, the compiler would detect cycles where there were none just because of other components that were compiled in parallel. Furthermore, the way ProtoView merging was triggered could result into early exits resulting in errors when trying to instantiate ProtoViews. Fixes #3206 Closes #3211
This commit is contained in:
parent
61b7703406
commit
5ec67ee2a7
|
@ -91,7 +91,6 @@ export class Compiler {
|
|||
private _appUrl: string;
|
||||
private _render: renderApi.RenderCompiler;
|
||||
private _protoViewFactory: ProtoViewFactory;
|
||||
private _protoViewsToBeMerged: AppProtoView[] = [];
|
||||
|
||||
/**
|
||||
* @private
|
||||
|
@ -141,55 +140,28 @@ export class Compiler {
|
|||
hostPvPromise =
|
||||
this._render.compileHost(directiveMetadata)
|
||||
.then((hostRenderPv) => {
|
||||
var protoView = this._protoViewFactory.createAppProtoViews(
|
||||
var protoViews = this._protoViewFactory.createAppProtoViews(
|
||||
componentBinding, hostRenderPv, [componentBinding]);
|
||||
this._compilerCache.setHost(componentType, protoView);
|
||||
return this._compileNestedProtoViews(hostRenderPv, protoView, componentType);
|
||||
return this._compileNestedProtoViews(protoViews, componentType, new Map());
|
||||
})
|
||||
.then((appProtoView) => {
|
||||
this._compilerCache.setHost(componentType, appProtoView);
|
||||
return appProtoView;
|
||||
});
|
||||
}
|
||||
return hostPvPromise.then(hostAppProtoView =>
|
||||
this._mergeUnmergedProtoViews().then(_ => hostAppProtoView.ref));
|
||||
return hostPvPromise.then(hostAppProtoView => hostAppProtoView.ref);
|
||||
}
|
||||
|
||||
private _mergeUnmergedProtoViews(): Promise<any> {
|
||||
var protoViewsToBeMerged = this._protoViewsToBeMerged;
|
||||
this._protoViewsToBeMerged = [];
|
||||
return PromiseWrapper.all(protoViewsToBeMerged.map((appProtoView) => {
|
||||
return this._render.mergeProtoViewsRecursively(
|
||||
this._collectMergeRenderProtoViews(appProtoView))
|
||||
.then((mergeResult: renderApi.RenderProtoViewMergeMapping) => {
|
||||
appProtoView.mergeMapping = new AppProtoViewMergeMapping(mergeResult);
|
||||
});
|
||||
}));
|
||||
}
|
||||
|
||||
private _collectMergeRenderProtoViews(
|
||||
appProtoView: AppProtoView): List<renderApi.RenderProtoViewRef | List<any>> {
|
||||
var result = [appProtoView.render];
|
||||
for (var i = 0; i < appProtoView.elementBinders.length; i++) {
|
||||
var binder = appProtoView.elementBinders[i];
|
||||
if (isPresent(binder.nestedProtoView)) {
|
||||
if (binder.hasStaticComponent() ||
|
||||
(binder.hasEmbeddedProtoView() && binder.nestedProtoView.isEmbeddedFragment)) {
|
||||
result.push(this._collectMergeRenderProtoViews(binder.nestedProtoView));
|
||||
} else {
|
||||
result.push(null);
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
private _compile(componentBinding: DirectiveBinding): Promise<AppProtoView>| AppProtoView {
|
||||
private _compile(componentBinding: DirectiveBinding,
|
||||
componentPath: Map<Type, AppProtoView>): Promise<AppProtoView>|
|
||||
AppProtoView {
|
||||
var component = <Type>componentBinding.key.token;
|
||||
var protoView = this._compilerCache.get(component);
|
||||
if (isPresent(protoView)) {
|
||||
// The component has already been compiled into an AppProtoView,
|
||||
// returns a plain AppProtoView, not wrapped inside of a Promise.
|
||||
// Needed for recursive components.
|
||||
// returns a plain AppProtoView, not wrapped inside of a Promise, for performance reasons.
|
||||
return protoView;
|
||||
}
|
||||
|
||||
var resultPromise = this._compiling.get(component);
|
||||
if (isPresent(resultPromise)) {
|
||||
// The component is already being compiled, attach to the existing Promise
|
||||
|
@ -212,16 +184,17 @@ export class Compiler {
|
|||
ListWrapper.map(directives, (directive) => this._bindDirective(directive)));
|
||||
|
||||
var renderTemplate = this._buildRenderTemplate(component, view, boundDirectives);
|
||||
resultPromise = this._render.compile(renderTemplate)
|
||||
resultPromise =
|
||||
this._render.compile(renderTemplate)
|
||||
.then((renderPv) => {
|
||||
var protoView = this._protoViewFactory.createAppProtoViews(
|
||||
var protoViews = this._protoViewFactory.createAppProtoViews(
|
||||
componentBinding, renderPv, boundDirectives);
|
||||
// Populate the cache before compiling the nested components,
|
||||
// so that components can reference themselves in their template.
|
||||
this._compilerCache.set(component, protoView);
|
||||
return this._compileNestedProtoViews(protoViews, component, componentPath);
|
||||
})
|
||||
.then((appProtoView) => {
|
||||
this._compilerCache.set(component, appProtoView);
|
||||
MapWrapper.delete(this._compiling, component);
|
||||
|
||||
return this._compileNestedProtoViews(renderPv, protoView, component);
|
||||
return appProtoView;
|
||||
});
|
||||
this._compiling.set(component, resultPromise);
|
||||
return resultPromise;
|
||||
|
@ -233,67 +206,83 @@ export class Compiler {
|
|||
return MapWrapper.values(directivesMap);
|
||||
}
|
||||
|
||||
private _compileNestedProtoViews(renderProtoView: renderApi.ProtoViewDto,
|
||||
appProtoView: AppProtoView,
|
||||
componentType: Type): Promise<AppProtoView> {
|
||||
private _compileNestedProtoViews(appProtoViews: AppProtoView[], componentType: Type,
|
||||
componentPath: Map<Type, AppProtoView>): Promise<AppProtoView> {
|
||||
var nestedPVPromises = [];
|
||||
this._loopComponentElementBinders(appProtoView, (parentPv, elementBinder: ElementBinder) => {
|
||||
componentPath = MapWrapper.clone(componentPath);
|
||||
if (appProtoViews[0].type === renderApi.ViewType.COMPONENT) {
|
||||
componentPath.set(componentType, appProtoViews[0]);
|
||||
}
|
||||
appProtoViews.forEach(appProtoView => {
|
||||
this._collectComponentElementBinders(appProtoView)
|
||||
.forEach((elementBinder: ElementBinder) => {
|
||||
var nestedComponent = elementBinder.componentDirective;
|
||||
var nestedComponentType = <Type>nestedComponent.key.token;
|
||||
var elementBinderDone =
|
||||
(nestedPv: AppProtoView) => { elementBinder.nestedProtoView = nestedPv; };
|
||||
var nestedCall = this._compile(nestedComponent);
|
||||
if (componentPath.has(nestedComponentType)) {
|
||||
// cycle...
|
||||
if (appProtoView.isEmbeddedFragment) {
|
||||
throw new BaseException(
|
||||
`<ng-content> is used within the recursive path of ${stringify(nestedComponentType)}`);
|
||||
} else if (appProtoView.type === renderApi.ViewType.COMPONENT) {
|
||||
throw new BaseException(
|
||||
`Unconditional component cycle in ${stringify(nestedComponentType)}`);
|
||||
} else {
|
||||
elementBinderDone(componentPath.get(nestedComponentType));
|
||||
}
|
||||
} else {
|
||||
var nestedCall = this._compile(nestedComponent, componentPath);
|
||||
if (isPromise(nestedCall)) {
|
||||
nestedPVPromises.push((<Promise<AppProtoView>>nestedCall).then(elementBinderDone));
|
||||
} else {
|
||||
elementBinderDone(<AppProtoView>nestedCall);
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
return PromiseWrapper.all(nestedPVPromises)
|
||||
.then((_) => {
|
||||
this._collectMergableProtoViews(appProtoView, componentType);
|
||||
return appProtoView;
|
||||
.then(_ => PromiseWrapper.all(
|
||||
appProtoViews.map(appProtoView => this._mergeProtoView(appProtoView))))
|
||||
.then(_ => appProtoViews[0]);
|
||||
}
|
||||
|
||||
private _mergeProtoView(appProtoView: AppProtoView): Promise<any> {
|
||||
if (appProtoView.type !== renderApi.ViewType.HOST &&
|
||||
appProtoView.type !== renderApi.ViewType.EMBEDDED) {
|
||||
return null;
|
||||
}
|
||||
return this._render.mergeProtoViewsRecursively(this._collectMergeRenderProtoViews(appProtoView))
|
||||
.then((mergeResult: renderApi.RenderProtoViewMergeMapping) => {
|
||||
appProtoView.mergeMapping = new AppProtoViewMergeMapping(mergeResult);
|
||||
});
|
||||
}
|
||||
|
||||
private _collectMergableProtoViews(appProtoView: AppProtoView, componentType: Type) {
|
||||
var isRecursive = false;
|
||||
private _collectMergeRenderProtoViews(
|
||||
appProtoView: AppProtoView): List<renderApi.RenderProtoViewRef | List<any>> {
|
||||
var result = [appProtoView.render];
|
||||
for (var i = 0; i < appProtoView.elementBinders.length; i++) {
|
||||
var binder = appProtoView.elementBinders[i];
|
||||
if (binder.hasStaticComponent()) {
|
||||
if (isBlank(binder.nestedProtoView.isRecursive)) {
|
||||
// cycle via a component. We are in the tail recursion,
|
||||
// so all components should have their isRecursive flag set already.
|
||||
isRecursive = true;
|
||||
break;
|
||||
}
|
||||
} else if (binder.hasEmbeddedProtoView()) {
|
||||
this._collectMergableProtoViews(binder.nestedProtoView, componentType);
|
||||
if (isPresent(binder.nestedProtoView)) {
|
||||
if (binder.hasStaticComponent() ||
|
||||
(binder.hasEmbeddedProtoView() && binder.nestedProtoView.isEmbeddedFragment)) {
|
||||
result.push(this._collectMergeRenderProtoViews(binder.nestedProtoView));
|
||||
} else {
|
||||
result.push(null);
|
||||
}
|
||||
}
|
||||
if (isRecursive) {
|
||||
if (appProtoView.isEmbeddedFragment) {
|
||||
throw new BaseException(
|
||||
`<ng-content> is used within the recursive path of ${stringify(componentType)}`);
|
||||
}
|
||||
if (appProtoView.type === renderApi.ViewType.COMPONENT) {
|
||||
throw new BaseException(`Unconditional component cycle in ${stringify(componentType)}`);
|
||||
}
|
||||
}
|
||||
if (appProtoView.type === renderApi.ViewType.EMBEDDED ||
|
||||
appProtoView.type === renderApi.ViewType.HOST) {
|
||||
this._protoViewsToBeMerged.push(appProtoView);
|
||||
}
|
||||
appProtoView.isRecursive = isRecursive;
|
||||
return result;
|
||||
}
|
||||
|
||||
private _loopComponentElementBinders(appProtoView: AppProtoView, callback: Function) {
|
||||
private _collectComponentElementBinders(appProtoView: AppProtoView): ElementBinder[] {
|
||||
var componentElementBinders = [];
|
||||
appProtoView.elementBinders.forEach((elementBinder) => {
|
||||
if (isPresent(elementBinder.componentDirective)) {
|
||||
callback(appProtoView, elementBinder);
|
||||
} else if (isPresent(elementBinder.nestedProtoView)) {
|
||||
this._loopComponentElementBinders(elementBinder.nestedProtoView, callback);
|
||||
componentElementBinders.push(elementBinder);
|
||||
}
|
||||
});
|
||||
return componentElementBinders;
|
||||
}
|
||||
|
||||
private _buildRenderTemplate(component, view, directives): renderApi.ViewDefinition {
|
||||
|
|
|
@ -160,7 +160,7 @@ export class ProtoViewFactory {
|
|||
|
||||
createAppProtoViews(hostComponentBinding: DirectiveBinding,
|
||||
rootRenderProtoView: renderApi.ProtoViewDto,
|
||||
allDirectives: List<DirectiveBinding>): AppProtoView {
|
||||
allDirectives: List<DirectiveBinding>): AppProtoView[] {
|
||||
var allRenderDirectiveMetadata =
|
||||
ListWrapper.map(allDirectives, directiveBinding => directiveBinding.metadata);
|
||||
var nestedPvsWithIndex = _collectNestedProtoViews(rootRenderProtoView);
|
||||
|
@ -184,7 +184,7 @@ export class ProtoViewFactory {
|
|||
}
|
||||
appProtoViews[pvWithIndex.index] = appProtoView;
|
||||
});
|
||||
return appProtoViews[0];
|
||||
return appProtoViews;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -261,8 +261,6 @@ export class AppProtoView {
|
|||
mergeMapping: AppProtoViewMergeMapping;
|
||||
ref: ProtoViewRef;
|
||||
|
||||
isRecursive: boolean = null;
|
||||
|
||||
constructor(public type: renderApi.ViewType, public isEmbeddedFragment: boolean,
|
||||
public render: renderApi.RenderProtoViewRef,
|
||||
public protoChangeDetector: ProtoChangeDetector,
|
||||
|
|
|
@ -374,7 +374,7 @@ export function main() {
|
|||
it('should cache compiled host components', inject([AsyncTestCompleter], (async) => {
|
||||
tplResolver.setView(MainComponent, new viewAnn.View({template: '<div></div>'}));
|
||||
var mainPv = createProtoView();
|
||||
var compiler = createCompiler([createRenderProtoView()], [rootProtoView, mainPv]);
|
||||
var compiler = createCompiler([createRenderProtoView([])], [rootProtoView, mainPv]);
|
||||
compiler.compileInHost(MainComponent)
|
||||
.then((protoViewRef) => {
|
||||
expect(internalProtoView(protoViewRef).elementBinders[0].nestedProtoView)
|
||||
|
@ -535,7 +535,8 @@ export function main() {
|
|||
it('should create host proto views', inject([AsyncTestCompleter], (async) => {
|
||||
tplResolver.setView(MainComponent, new viewAnn.View({template: '<div></div>'}));
|
||||
var rootProtoView =
|
||||
createProtoView([createComponentElementBinder(directiveResolver, MainComponent)]);
|
||||
createProtoView([createComponentElementBinder(directiveResolver, MainComponent)],
|
||||
renderApi.ViewType.HOST);
|
||||
var mainProtoView = createProtoView();
|
||||
var compiler = createCompiler([createRenderProtoView()], [rootProtoView, mainProtoView]);
|
||||
compiler.compileInHost(MainComponent)
|
||||
|
@ -694,9 +695,9 @@ class FakeProtoViewFactory extends ProtoViewFactory {
|
|||
}
|
||||
|
||||
createAppProtoViews(componentBinding: DirectiveBinding, renderProtoView: renderApi.ProtoViewDto,
|
||||
directives: List<DirectiveBinding>): AppProtoView {
|
||||
directives: List<DirectiveBinding>): AppProtoView[] {
|
||||
this.requests.push([componentBinding, renderProtoView, directives]);
|
||||
return ListWrapper.removeAt(this.results, 0);
|
||||
return collectEmbeddedPvs(ListWrapper.removeAt(this.results, 0));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -707,3 +708,16 @@ class MergedRenderProtoViewRef extends renderApi.RenderProtoViewRef {
|
|||
function originalRenderProtoViewRefs(appProtoView: AppProtoView) {
|
||||
return (<MergedRenderProtoViewRef>appProtoView.mergeMapping.renderProtoViewRef).originals;
|
||||
}
|
||||
|
||||
function collectEmbeddedPvs(pv: AppProtoView, target: AppProtoView[] = null): AppProtoView[] {
|
||||
if (isBlank(target)) {
|
||||
target = [];
|
||||
}
|
||||
target.push(pv);
|
||||
pv.elementBinders.forEach(elementBinder => {
|
||||
if (elementBinder.hasEmbeddedProtoView()) {
|
||||
collectEmbeddedPvs(elementBinder.nestedProtoView, target);
|
||||
}
|
||||
});
|
||||
return target;
|
||||
}
|
|
@ -66,10 +66,10 @@ export function main() {
|
|||
var varBindings = new Map();
|
||||
varBindings.set('a', 'b');
|
||||
var renderPv = createRenderProtoView([], null, varBindings);
|
||||
var appPv =
|
||||
var appPvs =
|
||||
protoViewFactory.createAppProtoViews(bindDirective(MainComponent), renderPv, []);
|
||||
expect(appPv.variableBindings.get('a')).toEqual('b');
|
||||
expect(appPv).toBeTruthy();
|
||||
expect(appPvs[0].variableBindings.get('a')).toEqual('b');
|
||||
expect(appPvs.length).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
Loading…
Reference in New Issue