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

+176
-34
lines changed

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

+18-4
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

+3
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

+4
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

+3
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

+43-1
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

+32-9
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

+1-2
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

+1-2
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

+2
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

+1
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());

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export function createFlatArray(expressions: o.Expression[]): o.Expression {
8888

8989
export function createPureProxy(fn: o.Expression, argCount: number, pureProxyProp: o.ReadPropExpr,
9090
view: CompileView) {
91-
view.fields.push(new o.ClassField(pureProxyProp.name, null, [o.StmtModifier.Private]));
91+
view.fields.push(new o.ClassField(pureProxyProp.name, null));
9292
var pureProxyId =
9393
argCount < Identifiers.pureProxies.length ? Identifiers.pureProxies[argCount] : null;
9494
if (isBlank(pureProxyId)) {

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

+4-7
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,8 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
128128
private _visitText(ast: TemplateAst, value: string, ngContentIndex: number,
129129
parent: CompileElement): o.Expression {
130130
var fieldName = `_text_${this.view.nodes.length}`;
131-
this.view.fields.push(new o.ClassField(fieldName,
132-
o.importType(this.view.genConfig.renderTypes.renderText),
133-
[o.StmtModifier.Private]));
131+
this.view.fields.push(
132+
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderText)));
134133
var renderNode = o.THIS_EXPR.prop(fieldName);
135134
var compileNode = new CompileNode(parent, this.view, this.view.nodes.length, renderNode, ast);
136135
var createRenderNode =
@@ -194,8 +193,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
194193
}
195194
var fieldName = `_el_${nodeIndex}`;
196195
this.view.fields.push(
197-
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderElement),
198-
[o.StmtModifier.Private]));
196+
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderElement)));
199197
this.view.createMethod.addStmt(o.THIS_EXPR.prop(fieldName).set(createRenderNodeExpr).toStmt());
200198

201199
var renderNode = o.THIS_EXPR.prop(fieldName);
@@ -257,8 +255,7 @@ class ViewBuilderVisitor implements TemplateAstVisitor {
257255
var nodeIndex = this.view.nodes.length;
258256
var fieldName = `_anchor_${nodeIndex}`;
259257
this.view.fields.push(
260-
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderComment),
261-
[o.StmtModifier.Private]));
258+
new o.ClassField(fieldName, o.importType(this.view.genConfig.renderTypes.renderComment)));
262259
this.view.createMethod.addStmt(
263260
o.THIS_EXPR.prop(fieldName)
264261
.set(ViewProperties.renderer.callMethod(

‎modules/angular2/src/facade/lang.dart

+1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ bool isString(Object obj) => obj is String;
2626
bool isFunction(Object obj) => obj is Function;
2727
bool isType(Object obj) => obj is Type;
2828
bool isStringMap(Object obj) => obj is Map;
29+
bool isStrictStringMap(Object obj) => obj is Map;
2930
bool isArray(Object obj) => obj is List;
3031
bool isPromise(Object obj) => obj is Future;
3132
bool isNumber(Object obj) => obj is num;

‎modules/angular2/src/facade/lang.ts

+5
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ export function isStringMap(obj: any): boolean {
138138
return typeof obj === 'object' && obj !== null;
139139
}
140140

141+
const STRING_MAP_PROTO = Object.getPrototypeOf({});
142+
export function isStrictStringMap(obj: any): boolean {
143+
return isStringMap(obj) && Object.getPrototypeOf(obj) === STRING_MAP_PROTO;
144+
}
145+
141146
export function isPromise(obj: any): boolean {
142147
return obj instanceof (<any>_global).Promise;
143148
}

‎modules/angular2/test/core/linker/view_injector_integration_spec.ts

+25
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,27 @@ export function main() {
314314
expect(d.dependency).toBeAnInstanceOf(SimpleDirective);
315315
}));
316316

317+
it('should support useValue with different values', fakeAsync(() => {
318+
var el = createComp('', tcb.overrideProviders(TestComp, [
319+
provide('numLiteral', {useValue: 0}),
320+
provide('boolLiteral', {useValue: true}),
321+
provide('strLiteral', {useValue: 'a'}),
322+
provide('null', {useValue: null}),
323+
provide('array', {useValue: [1]}),
324+
provide('map', {useValue: {'a': 1}}),
325+
provide('instance', {useValue: new TestValue('a')}),
326+
provide('nested', {useValue: [{'a': [1]}, new TestValue('b')]}),
327+
]));
328+
expect(el.inject('numLiteral')).toBe(0);
329+
expect(el.inject('boolLiteral')).toBe(true);
330+
expect(el.inject('strLiteral')).toBe('a');
331+
expect(el.inject('null')).toBe(null);
332+
expect(el.inject('array')).toEqual([1]);
333+
expect(el.inject('map')).toEqual({'a': 1});
334+
expect(el.inject('instance')).toEqual(new TestValue('a'));
335+
expect(el.inject('nested')).toEqual([{'a': [1]}, new TestValue('b')]);
336+
}));
337+
317338
it("should instantiate providers that have dependencies with SkipSelf", fakeAsync(() => {
318339
var el = createComp('<div simpleDirective><span someOtherDirective></span></div>',
319340
tcb.overrideProviders(
@@ -679,3 +700,7 @@ export function main() {
679700
});
680701
});
681702
}
703+
704+
class TestValue {
705+
constructor(public value: string) {}
706+
}

‎tools/compiler_cli/test/src/basic.ts

+3-8
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
1-
import {Component, Injectable} from 'angular2/core';
1+
import {Component, Inject} from 'angular2/core';
22
import {FORM_DIRECTIVES} from 'angular2/common';
33
import {MyComp} from './a/multiple_components';
44

5-
@Component({
6-
selector: 'basic',
7-
templateUrl: './basic.html',
8-
directives: [MyComp, FORM_DIRECTIVES],
9-
})
10-
@Injectable()
5+
@Component({selector: 'basic', templateUrl: './basic.html', directives: [MyComp, FORM_DIRECTIVES]})
116
export class Basic {
127
ctxProp: string;
13-
constructor() { this.ctxProp = 'initial value'; }
8+
constructor() { this.ctxProp = 'initiaValue'; }
149
}

0 commit comments

Comments
 (0)