Skip to content

Commit f114d6c

Browse files
tboschtbosch
committed
fix(compiler): fix cross view references and providers with useValue.
Before, we would create all fields in the generated views with visibility `private`. This does not work if an embedded view references a directive / element in a parent view. In Dart, this was no problem so far as it does not have a `private` modifier. Before, `useValue` in a provider did not work when doing offline compile, as so far the `MetadataResolver` was only used for jit mode. Now, `useValue` supports any kind of value that the static reflector can return. E.g. primitives, arrays, string maps, … Closes angular#8366
1 parent 163d80a commit f114d6c

17 files changed

Lines changed: 176 additions & 34 deletions

File tree

‎modules/angular2/src/compiler/metadata_resolver.ts‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {LifecycleHooks, LIFECYCLE_HOOKS_VALUES} from 'angular2/src/core/metadata
2424
import {reflector} from 'angular2/src/core/reflection/reflection';
2525
import {Injectable, Inject, Optional} from 'angular2/src/core/di';
2626
import {PLATFORM_DIRECTIVES, PLATFORM_PIPES} from 'angular2/src/core/platform_directives_and_pipes';
27-
import {MODULE_SUFFIX, sanitizeIdentifier} from './util';
27+
import {MODULE_SUFFIX, sanitizeIdentifier, ValueTransformer, visitValue} from './util';
2828
import {assertArrayOfStrings} from './assertions';
2929
import {getUrlScheme} from 'angular2/src/compiler/url_resolver';
3030
import {Provider} from 'angular2/src/core/di/provider';
@@ -314,9 +314,7 @@ export class CompileMetadataResolver {
314314
isPresent(provider.useClass) ?
315315
this.getTypeMetadata(provider.useClass, staticTypeModuleUrl(provider.useClass)) :
316316
null,
317-
useValue: isPresent(provider.useValue) ?
318-
new cpl.CompileIdentifierMetadata({runtime: provider.useValue}) :
319-
null,
317+
useValue: convertToCompileValue(provider.useValue),
320318
useFactory: isPresent(provider.useFactory) ?
321319
this.getFactoryMetadata(provider.useFactory,
322320
staticTypeModuleUrl(provider.useFactory)) :
@@ -417,3 +415,19 @@ function calcTemplateBaseUrl(reflector: ReflectorReader, type: any,
417415

418416
return reflector.importUri(type);
419417
}
418+
419+
// Only fill CompileIdentifierMetadata.runtime if needed...
420+
function convertToCompileValue(value: any): any {
421+
return visitValue(value, new _CompileValueConverter(), null);
422+
}
423+
424+
class _CompileValueConverter extends ValueTransformer {
425+
visitOther(value: any, context: any): any {
426+
if (isStaticType(value)) {
427+
return new cpl.CompileIdentifierMetadata(
428+
{name: value['name'], moduleUrl: staticTypeModuleUrl(value)});
429+
} else {
430+
return new cpl.CompileIdentifierMetadata({runtime: value});
431+
}
432+
}
433+
}

‎modules/angular2/src/compiler/output/dart_emitter.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,9 @@ class _DartEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisito
344344

345345
private _visitIdentifier(value: CompileIdentifierMetadata, typeParams: o.Type[],
346346
ctx: EmitterVisitorContext): void {
347+
if (isBlank(value.name)) {
348+
throw new BaseException(`Internal error: unknown identifier ${value}`);
349+
}
347350
if (isPresent(value.moduleUrl) && value.moduleUrl != this._moduleUrl) {
348351
var prefix = this.importsWithPrefixes.get(value.moduleUrl);
349352
if (isBlank(prefix)) {

‎modules/angular2/src/compiler/output/js_emitter.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
RegExpWrapper,
88
StringWrapper
99
} from 'angular2/src/facade/lang';
10+
import {BaseException} from 'angular2/src/facade/exceptions';
1011
import {OutputEmitter, EmitterVisitorContext} from './abstract_emitter';
1112
import {AbstractJsEmitterVisitor} from './abstract_js_emitter';
1213
import {getImportModulePath, ImportEnv} from './path_util';
@@ -34,6 +35,9 @@ class JsEmitterVisitor extends AbstractJsEmitterVisitor {
3435
constructor(private _moduleUrl: string) { super(); }
3536

3637
visitExternalExpr(ast: o.ExternalExpr, ctx: EmitterVisitorContext): any {
38+
if (isBlank(ast.value.name)) {
39+
throw new BaseException(`Internal error: unknown identifier ${ast.value}`);
40+
}
3741
if (isPresent(ast.value.moduleUrl) && ast.value.moduleUrl != this._moduleUrl) {
3842
var prefix = this.importsWithPrefixes.get(ast.value.moduleUrl);
3943
if (isBlank(prefix)) {

‎modules/angular2/src/compiler/output/ts_emitter.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,9 @@ class _TsEmitterVisitor extends AbstractEmitterVisitor implements o.TypeVisitor
311311

312312
private _visitIdentifier(value: CompileIdentifierMetadata, typeParams: o.Type[],
313313
ctx: EmitterVisitorContext): void {
314+
if (isBlank(value.name)) {
315+
throw new BaseException(`Internal error: unknown identifier ${value}`);
316+
}
314317
if (isPresent(value.moduleUrl) && value.moduleUrl != this._moduleUrl) {
315318
var prefix = this.importsWithPrefixes.get(value.moduleUrl);
316319
if (isBlank(prefix)) {

‎modules/angular2/src/compiler/util.ts‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
1-
import {IS_DART, StringWrapper, Math, isBlank} from 'angular2/src/facade/lang';
1+
import {
2+
IS_DART,
3+
StringWrapper,
4+
Math,
5+
isBlank,
6+
isArray,
7+
isStrictStringMap,
8+
isPrimitive
9+
} from 'angular2/src/facade/lang';
10+
import {StringMapWrapper} from 'angular2/src/facade/collection';
211

312
export var MODULE_SUFFIX = IS_DART ? '.dart' : '';
413

@@ -27,3 +36,36 @@ export function splitAtColon(input: string, defaultValues: string[]): string[] {
2736
export function sanitizeIdentifier(name: string): string {
2837
return StringWrapper.replaceAll(name, /\W/g, '_');
2938
}
39+
40+
export function visitValue(value: any, visitor: ValueVisitor, context: any): any {
41+
if (isArray(value)) {
42+
return visitor.visitArray(<any[]>value, context);
43+
} else if (isStrictStringMap(value)) {
44+
return visitor.visitStringMap(<{[key: string]: any}>value, context);
45+
} else if (isBlank(value) || isPrimitive(value)) {
46+
return visitor.visitPrimitive(value, context);
47+
} else {
48+
return visitor.visitOther(value, context);
49+
}
50+
}
51+
52+
export interface ValueVisitor {
53+
visitArray(arr: any[], context: any): any;
54+
visitStringMap(map: {[key: string]: any}, context: any): any;
55+
visitPrimitive(value: any, context: any): any;
56+
visitOther(value: any, context: any): any;
57+
}
58+
59+
export class ValueTransformer implements ValueVisitor {
60+
visitArray(arr: any[], context: any): any {
61+
return arr.map(value => visitValue(value, this, context));
62+
}
63+
visitStringMap(map: {[key: string]: any}, context: any): any {
64+
var result = {};
65+
StringMapWrapper.forEach(map,
66+
(value, key) => { result[key] = visitValue(value, this, context); });
67+
return result;
68+
}
69+
visitPrimitive(value: any, context: any): any { return value; }
70+
visitOther(value: any, context: any): any { return value; }
71+
}

‎modules/angular2/src/compiler/view_compiler/compile_element.ts‎

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import {BaseException} from 'angular2/src/facade/exceptions';
12
import * as o from '../output/output_ast';
23
import {Identifiers, identifierToken} from '../identifiers';
34
import {InjectMethodVars} from './constants';
@@ -18,6 +19,7 @@ import {
1819
import {getPropertyInView, createDiTokenExpression, injectFromViewParentInjector} from './util';
1920
import {CompileQuery, createQueryList, addQueryToTokenMap} from './compile_query';
2021
import {CompileMethod} from './compile_method';
22+
import {ValueTransformer, visitValue} from '../util';
2123

2224
export class CompileNode {
2325
constructor(public parent: CompileElement, public view: CompileView, public nodeIndex: number,
@@ -72,6 +74,7 @@ export class CompileElement extends CompileNode {
7274
private _createAppElement() {
7375
var fieldName = `_appEl_${this.nodeIndex}`;
7476
var parentNodeIndex = this.isRootElement() ? null : this.parent.nodeIndex;
77+
// private is fine here as no child view will reference an AppElement
7578
this.view.fields.push(new o.ClassField(fieldName, o.importType(Identifiers.AppElement),
7679
[o.StmtModifier.Private]));
7780
var statement = o.THIS_EXPR.prop(fieldName)
@@ -140,13 +143,7 @@ export class CompileElement extends CompileNode {
140143
return o.importExpr(provider.useClass)
141144
.instantiate(depsExpr, o.importType(provider.useClass));
142145
} else {
143-
if (provider.useValue instanceof CompileIdentifierMetadata) {
144-
return o.importExpr(provider.useValue);
145-
} else if (provider.useValue instanceof o.Expression) {
146-
return provider.useValue;
147-
} else {
148-
return o.literal(provider.useValue);
149-
}
146+
return _convertValueToOutputAst(provider.useValue);
150147
}
151148
});
152149
var propName = `_${resolvedProvider.token.name}_${this.nodeIndex}_${this._instances.size}`;
@@ -379,11 +376,11 @@ function createProviderProperty(propName: string, provider: ProviderAst,
379376
type = o.DYNAMIC_TYPE;
380377
}
381378
if (isEager) {
382-
view.fields.push(new o.ClassField(propName, type, [o.StmtModifier.Private]));
379+
view.fields.push(new o.ClassField(propName, type));
383380
view.createMethod.addStmt(o.THIS_EXPR.prop(propName).set(resolvedProviderValueExpr).toStmt());
384381
} else {
385382
var internalField = `_${propName}`;
386-
view.fields.push(new o.ClassField(internalField, type, [o.StmtModifier.Private]));
383+
view.fields.push(new o.ClassField(internalField, type));
387384
var getter = new CompileMethod(view);
388385
getter.resetDebugInfo(compileElement.nodeIndex, compileElement.sourceAst);
389386
// Note: Equals is important for JS so that it also checks the undefined case!
@@ -402,3 +399,29 @@ class _QueryWithRead {
402399
this.read = isPresent(query.meta.read) ? query.meta.read : match;
403400
}
404401
}
402+
403+
function _convertValueToOutputAst(value: any): o.Expression {
404+
return visitValue(value, new _ValueOutputAstTransformer(), null);
405+
}
406+
407+
class _ValueOutputAstTransformer extends ValueTransformer {
408+
visitArray(arr: any[], context: any): o.Expression {
409+
return o.literalArr(arr.map(value => visitValue(value, this, context)));
410+
}
411+
visitStringMap(map: {[key: string]: any}, context: any): o.Expression {
412+
var entries = [];
413+
StringMapWrapper.forEach(
414+
map, (value, key) => { entries.push([key, visitValue(value, this, context)]); });
415+
return o.literalMap(entries);
416+
}
417+
visitPrimitive(value: any, context: any): o.Expression { return o.literal(value); }
418+
visitOther(value: any, context: any): o.Expression {
419+
if (value instanceof CompileIdentifierMetadata) {
420+
return o.importExpr(value);
421+
} else if (value instanceof o.Expression) {
422+
return value;
423+
} else {
424+
throw new BaseException(`Illegal state: Don't now how to compile value ${value}`);
425+
}
426+
}
427+
}

‎modules/angular2/src/compiler/view_compiler/compile_pipe.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ export class CompilePipe {
2929
}
3030
return injectFromViewParentInjector(diDep.token, false);
3131
});
32-
this.view.fields.push(new o.ClassField(this.instance.name, o.importType(this.meta.type),
33-
[o.StmtModifier.Private]));
32+
this.view.fields.push(new o.ClassField(this.instance.name, o.importType(this.meta.type)));
3433
this.view.createMethod.resetDebugInfo(null, null);
3534
this.view.createMethod.addStmt(o.THIS_EXPR.prop(this.instance.name)
3635
.set(o.importExpr(this.meta.type).instantiate(deps))

‎modules/angular2/src/compiler/view_compiler/compile_query.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,7 @@ function mapNestedViews(declarationAppElement: o.Expression, view: CompileView,
9797

9898
export function createQueryList(query: CompileQueryMetadata, directiveInstance: o.Expression,
9999
propertyName: string, compileView: CompileView): o.Expression {
100-
compileView.fields.push(new o.ClassField(propertyName, o.importType(Identifiers.QueryList),
101-
[o.StmtModifier.Private]));
100+
compileView.fields.push(new o.ClassField(propertyName, o.importType(Identifiers.QueryList)));
102101
var expr = o.THIS_EXPR.prop(propertyName);
103102
compileView.createMethod.addStmt(o.THIS_EXPR.prop(propertyName)
104103
.set(o.importExpr(Identifiers.QueryList).instantiate([]))

‎modules/angular2/src/compiler/view_compiler/event_binder.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export class CompileEventListener {
7777
(<o.Statement[]>[markPathToRootStart.callMethod('markPathToRootAsCheckOnce', []).toStmt()])
7878
.concat(this._method.finish())
7979
.concat([new o.ReturnStatement(resultExpr)]);
80+
// private is fine here as no child view will reference the event handler...
8081
this.compileElement.view.eventHandlerMethods.push(new o.ClassMethod(
8182
this._methodName, [this._eventParam], stmts, o.BOOL_TYPE, [o.StmtModifier.Private]));
8283
}
@@ -95,6 +96,7 @@ export class CompileEventListener {
9596
}
9697
var disposable = o.variable(`disposable_${this.compileElement.view.disposables.length}`);
9798
this.compileElement.view.disposables.push(disposable);
99+
// private is fine here as no child view will reference the event handler...
98100
this.compileElement.view.createMethod.addStmt(
99101
disposable.set(listenExpr).toDeclStmt(o.FUNCTION_TYPE, [o.StmtModifier.Private]));
100102
}

‎modules/angular2/src/compiler/view_compiler/property_binder.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ function bind(view: CompileView, currValExpr: o.ReadVarExpr, fieldExpr: o.ReadPr
4343
return;
4444
}
4545

46+
// private is fine here as no child view will reference the cached value...
4647
view.fields.push(new o.ClassField(fieldExpr.name, null, [o.StmtModifier.Private]));
4748
view.createMethod.addStmt(
4849
o.THIS_EXPR.prop(fieldExpr.name).set(o.importExpr(Identifiers.uninitialized)).toStmt());

0 commit comments

Comments
 (0)